RE: [PATCH RESEND 1/4] ARM: OMAP2+: AM33XX: Add tps65910 device tree data
On Fri, Jul 20, 2012 at 17:08:06, Mark Brown wrote: > On Fri, Jul 20, 2012 at 11:27:36AM +, AnilKumar, Chimata wrote: > > On Fri, Jul 20, 2012 at 15:29:36, Mark Brown wrote: > > > > Every regulator here has a rather large voltage range specified with no > > > consumers added. Are you sure these voltage ranges make sense in your > > > design and you've not just cut'n'pasted the entire voltage range that > > > your regulator supports without reference to what your board can do? > > > tps65217.dtsi is a generic file to be used by the SoCs so these constraints > > were taken from the regulator itself. SoC specific limits can be added in > > SoC specific .dts file to tighten the constraints to require limit. I have > > tested the driver with this approach. > > No, this is not a sane approach. You've no idea if any of these > settings are safe or sane for the board. Boards should enable things > they know are safe, not remove those they know are broken. > Unsterstood, I will send v2 with constraints updated. Regards AnilKumar-- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
am3517: USB state in 3.5-rc6
After getting MMC working I now have following issues: both musb and EHCI make problems. musb: musb-hdrc: version 6.0, ?dma?, otg (peripheral+host) musb-am35x musb-am35x: failed to get clock musb-am35x: probe of musb-am35x failed with error -2 EHCI with attached smsc9514: LR is at __func__.12097+0x81924/0xda0c4 pc : []lr : []psr: 6013 sp : cf92dea8 ip : fp : 0002 r10: cfb07000 r9 : 0001 r8 : 0003 r7 : cfb11800 r6 : 0032 r5 : r4 : cfb2cc00 r3 : c05919a0 r2 : c05851fc r1 : c0585220 r0 : cfb2cc68 Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment kernel Control: 10c5387d Table: 80004019 DAC: 0015 Process khubd (pid: 385, stack limit = 0xcf92c2f0) Stack: (0xcf92dea8 to 0xcf92e000) dea0: 0002 0001 cfb07000 c006eb24 dec0: a013 0003 cfb11c00 c0496a9c cfb2cc00 cfb07000 dee0: cfb2cc00 cfb11800 cfb11c18 0001 cfb11c00 0002 c02e6e0c df00: 0501 c0585740 c04959c0 0001 2093 cfb07008 cfb1189c df20: cfb11400 cfb07078 cfb07070 cfb07008 cfb07000 cfb11800 cfb11420 df40: cfb11800 cfb11420 cfb11c00 0009 cf92df84 c00590e4 cf92c000 df60: cf8b8100 c0051304 cf92df6c cf92df6c 0501 c006eb24 df80: 6013 cf82df18 cf92dfb4 c02e66e4 dfa0: c00508dc cf92c000 0001 dead4ead dfc0: c06894d8 c054b8a4 cf92dfd8 cf92dfd8 dfe0: cf82df18 cf82df18 c0050850 c00146f0 0013 c00146f0 [] (hub_port_init+0x14c/0x97c) from [] (hub_thread+0x728/0x13b0) [] (hub_thread+0x728/0x13b0) from [] (kthread+0x8c/0x98) [] (kthread+0x8c/0x98) from [] (kernel_thread_exit+0x0/0x8) Code: e59f17fc e592c080 e59f27f8 11a0200e (e59cc000) ---[ end trace 56f9d1e7cf52e660 ]--- Yegor -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 1/4] ARM: OMAP3: cpuidle: Remove unused MPU OSWR support code
Hi Paul, On Friday 20 July 2012 11:55 PM, Paul Walmsley wrote: On Fri, 20 Jul 2012, Rajendra Nayak wrote: We do not support MPU OSWR on OMAP3. Get rid of the complex/multiple save_state handling in omap_sram_idle() and just use 2 save_state definitions save_state = 1, all logic and memory lost, MPU hits OFF save_state = 0, nothing lost, MPU hits CSWR or shallower state Signed-off-by: Rajendra Nayak The code is certainly simpler, but I recall seeing patchsets in the past that implemented OSWR on OMAP3. (Not sure which powerdomains it was implemented for.) Do you know what the status of those patchsets are? OMAP3 supports OSWR for MPU/IVA/CORE and PER powerdomains. Though we did have some OSWR implementations in our internal trees, we never ended up using them effectively, mainly because the latencies for OSWR as compared to OFF were very close on OMAP3 and the power savings from OFF (given we could hit 0v) significantly higher. OSWR on OMAP4 was hence reworked to keep much more retained in OSWR and make it, in latency terms, something _in_between_ a CSWR and OFF. Someone at some point (I don't recall and it could have even been me) would have tried to upstream the OSWR support from our internal trees, but as of now I don't know if anyone is trying to push or interested in OSWR support for OMAP3 anymore. regards, Rajendra - Paul -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] PWM: EHRPWM: PWM driver support for EHRPWM.
On Fri, Jul 13, 2012 at 03:05:02PM +0530, Philip, Avinash wrote: > Enhanced high resolution PWM module (EHRPWM) hardware can be used to > generate PWM output over 2 channels. This commit adds PWM driver support > for EHRPWM device present on AM33XX SOC. Current implementation supports > simple PWM functionality. > > Signed-off-by: Philip, Avinash > Reviewed-by: Vaibhav Bedia So this driver is very similar to the ECAP one and pretty much all the comments apply to this as well. Some additional comments below. > --- > :100644 100644 f20b8f2... ad62d7a... Mdrivers/pwm/Kconfig > :100644 100644 7dd90ec... 636a1d6... Mdrivers/pwm/Makefile > :00 100644 000... 985d334... Adrivers/pwm/pwm-ehrpwm.c > drivers/pwm/Kconfig | 10 + > drivers/pwm/Makefile |1 + > drivers/pwm/pwm-ehrpwm.c | 476 > ++ > 3 files changed, 487 insertions(+), 0 deletions(-) > > diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig > index f20b8f2..ad62d7a 100644 > --- a/drivers/pwm/Kconfig > +++ b/drivers/pwm/Kconfig > @@ -95,4 +95,14 @@ config PWM_ECAP > To compile this driver as a module, choose M here: the module > will be called pwm_ecap. > > +config PWM_EHRPWM > + tristate "EHRPWM PWM support" > + depends on SOC_AM33XX > + help > + PWM driver support for the EHRPWM controller found on AM33XX > + TI SOC > + > + To compile this driver as a module, choose M here: the module > + will be called pwm-ehrpwm. > + Maybe it would be useful to prefix the names with AM33XX here. ECAP and EHRPWM are sort of generic and may have name clashes in the future. > +#define PWM_CHANNEL 2 /* EHRPWM channels */ I'd say you can just replace the one occurrence of this with the literal 2. If you still want to have the symbolic name, then I'd suggest to call it something like NUM_PWM_CHANNELS to make its meaning more obvious. > +static int __devexit ehrpwm_pwm_remove(struct platform_device *pdev) > +{ > + struct ehrpwm_pwm_chip *pc = platform_get_drvdata(pdev); > + > + if (WARN_ON(!pc)) > + return -ENODEV; > + > + pm_runtime_disable(&pdev->dev); > + pwmchip_remove(&pc->chip); > + return 0; > +} I forgot to mention this for ECAP, but you need to check the return value of pwmchip_remove() because there are situations where it can actually fail. Thierry pgp2Eaddj6Inw.pgp Description: PGP signature
Re: [PATCH] PWM: Add support for configuring polarity of PWM
On Wed, Jul 18, 2012 at 06:24:13PM +0530, Philip, Avinash wrote: > Duty cycle inversion of PWM wave should achieved through PWM polarity > inversion. Also polarity of PWM wave should configurable from slave > drivers, Actually, I don't think that duty cycle inversion *should* be achieved through polarity inversion. But it is true that the same effect *can* be achieved by inverting the polarity. > Configure polarity > 1. PWM_POLARITY_NORMAL -> duty ns defines ON period of PWM wave > 2. PWM_POLARITY_INVERSE -> duty ns defines OFF period of PWM wave. Similarly, this text describes how polarity inversion can be used to simular duty cycle inversion. I think you should describe that this change adds support for configuring the PWM polarity. If you absolutely must note that it can be used to simulate the duty cycle inversion, then you can give it as an example below the actual description. > This patch adds support for configuring PWM polarity in PWM frame work. "framework" is one word. > > Signed-off-by: Philip, Avinash > --- > Configuring polarity to be done with PWM device disabled, if not > failure reported. > > If PWM device is enabled while configuring polarity, disabling and > re_enabling make it complex. Whoever uses this API has to taken > care of the basic rules. These comments belong in the commit message because they are very useful information about the change that you introduce. They probably belong somewhere in the code as well. > Discussions related to this can found at > http://www.spinics.net/lists/kernel/msg1372110.html Here's my proposal for a revised commit message: pwm: Add support for configuring the PWM polarity Some hardware supports inverting the polarity of the PWM signal. This commit adds support to the PWM framework to allow users of the PWM API to configure the polarity. Note that in order to reduce complexity, changing the polarity of a PWM signal is only allowed while the PWM is disabled. A practical example where this can prove useful is to simulate inversion of the duty cycle. While inversion of polarity and duty cycle are not exactly the same, the differences for most use-cases are negligible. > :100644 100644 ecb7690... 24d5495... Mdrivers/pwm/core.c > :100644 100644 21d076c... 2e4e960... Minclude/linux/pwm.h > drivers/pwm/core.c | 32 > include/linux/pwm.h | 15 +++ > 2 files changed, 47 insertions(+), 0 deletions(-) > > diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c > index ecb7690..24d5495 100644 > --- a/drivers/pwm/core.c > +++ b/drivers/pwm/core.c > @@ -379,6 +379,38 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, int > period_ns) > EXPORT_SYMBOL_GPL(pwm_config); > > /** > + * pwm_set_polarity() - change PWM device Polarity Maybe: "pwm_set_polarity() - configure the polarity of a PWM signal" > + * @pwm: PWM device > + * @polarity: Configure polarity of PWM "new polarity of the PWM signal" > + * > + * Polarity to be configured with PWM device disabled. "Note that the polarity cannot be configured while the PWM device is enabled." > + */ > +int pwm_set_polarity(struct pwm_device *pwm, int polarity) The polarity parameter should be an enum pwm_polarity, see below. > +{ > + int pwm_flags = PWM_POLARITY_NORMAL; I don't think this is needed. > + > + if (!pwm || !pwm->chip->ops->set_polarity) > + return -EINVAL; I'd prefer -ENOSYS if .set_polarity is not implemented, so this check should probably be split up: if (!pwm || !pwm->chip || !pwm->chip->ops) return -EINVAL; if (!pwm->chip->ops->set_polarity) return -ENOSYS; > + > + if (test_bit(PWMF_ENABLED, &pwm->flags)) { > + dev_err(pwm->chip->dev, > + "Polarity configuration Failed!, PWM device enabled\n"); > + return -EBUSY; > + } Maybe something like: "polarity cannot be configured while PWM device is enabled"? Though I'm not sure the error message is all that useful. I'd expect the user driver to handle -EBUSY specially. > + > + if (polarity == PWM_POLARITY_INVERSE) > + pwm_flags = PWM_POLARITY_INVERSE; > + > + if (!pwm_flags) > + clear_bit(PWMF_POLARITY_INVERSE, &pwm->flags); > + else > + set_bit(PWMF_POLARITY_INVERSE, &pwm->flags); You can make this decision based on the value of polarity, no need for the additional variable. Also as I mention below, maybe this flag isn't all that useful. > + > + return pwm->chip->ops->set_polarity(pwm->chip, pwm, polarity); > +} > +EXPORT_SYMBOL_GPL(pwm_set_polarity); > + > +/** > * pwm_enable() - start a PWM output toggling > * @pwm: PWM device > */ > diff --git a/include/linux/pwm.h b/include/linux/pwm.h > index 21d076c..2e4e960 100644 > --- a/include/linux/pwm.h > +++ b/include/linux/pwm.h > @@ -21,6 +21,16 @@ void pwm_free(s
Re: [PATCH] OMAP: USB : Fix the EHCI enumeration and core retention issue
Hi, On 06/22/2012 05:43 PM, Munegowda, Keshava wrote: > On Fri, Jun 22, 2012 at 7:41 PM, Kevin Hilman wrote: >> "Munegowda, Keshava" writes: >> >> [...] >> >> >> You are not reading what I write. >> >> To repeat: your patch fixes the oops during boot, and the suspend hang >> and now I see CORE hit retention in *suspend*. > > thanks ! > >> >> However, CORE does still not hit retention during *idle*. > > here is the problem. > > usb host retention in idle is not supported till now. > in current code, usb host cuts clock only in driver suspend not in bus > suspend ( auto suspend). > usb host driver need to use the io daisy chain framework through io wakeup. > I will post the patches once ehci remote wakeup features stabilized in > omap3, omap4 and omap5 too. > We are talking about CORE retention support during idle. How is IO daisy chaining related to that? Doesn't IO daisy chain only apply when device hits OFF? regards, -roger -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] ARM: OMAP4: twl-common: Support for additional devices on i2c1 bus
Hi Tony, On 07/13/2012 10:38 AM, Peter Ujfalusi wrote: > On OMAP4 the i2c1 bus is dedicated for the PMIC and audio related devices. > Manufacturers can opt to use different codec than twl6040 and also can add > audio related IC to the bus (external amplifier for example on SDP4430). > > Make it possible to add differnet set of additional devices to i2c1 bus on > OMAP4 boards. Would it be possible to queue this patch for 3.6? Thank you, Péter > Signed-off-by: Peter Ujfalusi > --- > > Changes since v1: > Generated against l-o/master > > Regards, > Peter > > arch/arm/mach-omap2/board-4430sdp.c| 12 +- > arch/arm/mach-omap2/board-omap4panda.c | 12 +- > arch/arm/mach-omap2/twl-common.c | 35 +-- > arch/arm/mach-omap2/twl-common.h |3 +- > 4 files changed, 32 insertions(+), 30 deletions(-) > > diff --git a/arch/arm/mach-omap2/board-4430sdp.c > b/arch/arm/mach-omap2/board-4430sdp.c > index ad8a7d9..26466f2 100644 > --- a/arch/arm/mach-omap2/board-4430sdp.c > +++ b/arch/arm/mach-omap2/board-4430sdp.c > @@ -547,6 +547,14 @@ static struct twl6040_platform_data twl6040_data = { > .irq_base = TWL6040_CODEC_IRQ_BASE, > }; > > +static struct i2c_board_info __initdata sdp4430_i2c_1_boardinfo[] = { > + { > + I2C_BOARD_INFO("twl6040", 0x4b), > + .irq = OMAP44XX_IRQ_SYS_2N, > + .platform_data = &twl6040_data, > + }, > +}; > + > static struct twl4030_platform_data sdp4430_twldata = { > /* Regulators */ > .vusim = &sdp4430_vusim, > @@ -580,8 +588,8 @@ static int __init omap4_i2c_init(void) > TWL_COMMON_REGULATOR_CLK32KG | > TWL_COMMON_REGULATOR_V1V8 | > TWL_COMMON_REGULATOR_V2V1); > - omap4_pmic_init("twl6030", &sdp4430_twldata, > - &twl6040_data, OMAP44XX_IRQ_SYS_2N); > + omap4_pmic_init("twl6030", &sdp4430_twldata, sdp4430_i2c_1_boardinfo, > + ARRAY_SIZE(sdp4430_i2c_1_boardinfo)); > omap_register_i2c_bus(2, 400, NULL, 0); > omap_register_i2c_bus(3, 400, sdp4430_i2c_3_boardinfo, > ARRAY_SIZE(sdp4430_i2c_3_boardinfo)); > diff --git a/arch/arm/mach-omap2/board-omap4panda.c > b/arch/arm/mach-omap2/board-omap4panda.c > index b627cdc..5becba2 100644 > --- a/arch/arm/mach-omap2/board-omap4panda.c > +++ b/arch/arm/mach-omap2/board-omap4panda.c > @@ -266,6 +266,14 @@ static struct twl6040_platform_data twl6040_data = { > .irq_base = TWL6040_CODEC_IRQ_BASE, > }; > > +static struct i2c_board_info __initdata panda_i2c_1_boardinfo[] = { > + { > + I2C_BOARD_INFO("twl6040", 0x4b), > + .irq = OMAP44XX_IRQ_SYS_2N, > + .platform_data = &twl6040_data, > + }, > +}; > + > /* Panda board uses the common PMIC configuration */ > static struct twl4030_platform_data omap4_panda_twldata; > > @@ -293,8 +301,8 @@ static int __init omap4_panda_i2c_init(void) > TWL_COMMON_REGULATOR_CLK32KG | > TWL_COMMON_REGULATOR_V1V8 | > TWL_COMMON_REGULATOR_V2V1); > - omap4_pmic_init("twl6030", &omap4_panda_twldata, > - &twl6040_data, OMAP44XX_IRQ_SYS_2N); > + omap4_pmic_init("twl6030", &omap4_panda_twldata, panda_i2c_1_boardinfo, > + ARRAY_SIZE(panda_i2c_1_boardinfo)); > omap_register_i2c_bus(2, 400, NULL, 0); > /* >* Bus 3 is attached to the DVI port where devices like the pico DLP > diff --git a/arch/arm/mach-omap2/twl-common.c > b/arch/arm/mach-omap2/twl-common.c > index 3882f3c..d52be85 100644 > --- a/arch/arm/mach-omap2/twl-common.c > +++ b/arch/arm/mach-omap2/twl-common.c > @@ -39,16 +39,6 @@ static struct i2c_board_info __initdata > pmic_i2c_board_info = { > .flags = I2C_CLIENT_WAKE, > }; > > -static struct i2c_board_info __initdata omap4_i2c1_board_info[] = { > - { > - .addr = 0x48, > - .flags = I2C_CLIENT_WAKE, > - }, > - { > - I2C_BOARD_INFO("twl6040", 0x4b), > - }, > -}; > - > #if defined(CONFIG_ARCH_OMAP3) || defined(CONFIG_ARCH_OMAP4) > static int twl_set_voltage(void *data, int target_uV) > { > @@ -77,30 +67,25 @@ void __init omap_pmic_init(int bus, u32 clkrate, > > void __init omap4_pmic_init(const char *pmic_type, > struct twl4030_platform_data *pmic_data, > - struct twl6040_platform_data *twl6040_data, int twl6040_irq) > + struct i2c_board_info *devices, int nr_devices) > { > /* PMIC part*/ > omap_mux_init_signal("sys_nirq1", OMAP_PIN_INPUT_PULLUP | > OMAP_PIN_OFF_WAKEUPENABLE); > - strncpy(omap4_i2c1_board_info[0].type, pmic_type, > - sizeof(omap4_i2c1_board_info[0].type)); > - omap4_i2c1_board_info[0].irq = OMAP44XX_IRQ_SYS_1N; > - omap4_i2c1_board_info[0].p
RE: [PATCH 1/2] PWM: ECAP: PWM driver support for ECAP APWM.
On Mon, Jul 23, 2012 at 12:22:35, Thierry Reding wrote: > On Fri, Jul 13, 2012 at 03:05:01PM +0530, Philip, Avinash wrote: [snip] > > obj-$(CONFIG_PWM_VT8500) += pwm-vt8500.o > > +obj-$(CONFIG_PWM_ECAP) += pwm-ecap.o > > Both the Kconfig and Makefile should have the entries ordered > alphabetically. Ok. I will correct it. > [snip] > > +/* ECAP registers and bits definitions */ > > +#define TSCTR 0x00 > > +#define CTRPHS 0x04 > > +#define CAP1 0x08 > > +#define CAP2 0x0C > > +#define CAP3 0x10 > > +#define CAP4 0x14 > > +#define ECCTL1 0x28 > > These registers are not used. I guess there is some use to list all > registers here but on the other hand the majority are unused so they > just clutter the driver. > > > +#define ECCTL2_APWM_POL_LOWBIT(10) > > This bit is never used. > > > +#define ECEINT 0x2C > > +#define ECFLG 0x2E > > +#define ECCLR 0x30 > > +#define REVID 0x5c > > These are unused as well. Ok. I will remove it. These are been placed in future enhancement. > > > + > > +#define DRIVER_NAME"ecap" > > You only use this once when defining the struct platform_driver, so > maybe you can just drop it. In the v2 patch, I am planning to use same in platform_get_resource_byname(). Here I will use this define to search resources. > > > + > > +struct ecap_pwm_chip { > > + struct pwm_chip chip; > > + unsigned intclk_rate; > > + void __iomem*mmio_base; > > + int pwm_period_ns; > > + int pwm_duty_ns; > > +}; > > The pwm_period_ns and pwm_duty_ns don't seem to be used at all. Can they > be dropped? Ok. I will remove it. > > > + /* > > +* Enable 'Free run Time stamp counter mode' to start counter > > +* and 'APWM mode' to enable APWM output > > +*/ > > + reg_val = readw(pc->mmio_base + ECCTL2); > > + reg_val |= ECCTL2_TSCTR_FREERUN | ECCTL2_APWM_MODE; > > You already set the APWM_MODE bit in .config(), why is it needed here > again? Seeing that .disable() clears the bit as well, perhaps leaving it > clear in .config() is the better option. Clearing APWM mode in .disable() ensures PWM low output (i.e., PWM pin = low in idle state). The Period & Compare registers are updated from Shadow registers (CAP1 & CAP2) During PWM configuration. To enable loading from Shadow registers, APWM mode should be set. The same is done in .config(). > > > +} > > + > > +static void ecap_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm) > > +{ > > + if (test_bit(PWMF_ENABLED, &pwm->flags)) { > > + dev_warn(chip->dev, "Removing PWM device without disabling\n"); > > + pm_runtime_put_sync(chip->dev); > > + } > > +} > > I wonder whether we want to have something like this in the PWM core at > some point. Maybe we should call .disable() automatically when they are > freed, or alternatively return -EBUSY if a PWM is freed but still > enabled. I think I prefer the latter. For now we can leave this in, > because I don't want to make any such core changes for 3.6 now that the > merge window is open. OK Thanks. > > > + > > +static struct pwm_ops ecap_pwm_ops = { > > + .free = ecap_pwm_free, > > + .config = ecap_pwm_config, > > + .enable = ecap_pwm_enable, > > + .disable= ecap_pwm_disable, > > + .owner = THIS_MODULE, > > +}; > > This should be "static const pwm_ops ...". Ok I will correct it. > > > + > > +static int __devinit ecap_pwm_probe(struct platform_device *pdev) > > +{ > > + int ret; > > + struct resource *r; > > + struct clk *clk; > > + struct ecap_pwm_chip *pc; > > + > > + pc = devm_kzalloc(&pdev->dev, sizeof(*pc), GFP_KERNEL); > > + if (!pc) { > > + dev_err(&pdev->dev, "failed to allocate memory\n"); > > + return -ENOMEM; > > + } > > + > > + clk = devm_clk_get(&pdev->dev, "fck"); > > + if (IS_ERR(clk)) { > > + dev_err(&pdev->dev, "failed to get clock\n"); > > + return PTR_ERR(clk); > > + } > > + > > + pc->clk_rate = clk_get_rate(clk); > > + if (!pc->clk_rate) { > > + dev_err(&pdev->dev, "failed to get clock rate\n"); > > + return -EINVAL; > > + } > > + > > + pc->chip.dev = &pdev->dev; > > + pc->chip.ops = &ecap_pwm_ops; > > + pc->chip.base = -1; > > + pc->chip.npwm = 1; > > The cover letter mentions that the AM335x has 3 instances of the ECAP. > Is there any chance that they can be unified in one driver instance > (i.e. .npwm = 3?). No. All instances have separate resources (clocks, memory ..). > > > + > > + r = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ecap_reg"); > > + if (r == NULL) { > > You use "if (!ptr)" everywhere else, maybe you should make this > consistent? Ok > Also the pla
[PATCH 09/25] i2c/i2c-omap: add a const qualifier
This prepares *of_device_id.data becoming const. Without this change the following warning would occur: drivers/i2c/busses/i2c-omap.c: In function 'omap_i2c_probe': drivers/i2c/busses/i2c-omap.c:1025: warning: assignment discards qualifiers from pointer target type Signed-off-by: Uwe Kleine-König --- drivers/i2c/busses/i2c-omap.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index 801df60..4fc585d 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -987,7 +987,8 @@ omap_i2c_probe(struct platform_device *pdev) struct omap_i2c_dev *dev; struct i2c_adapter *adap; struct resource *mem, *irq, *ioarea; - struct omap_i2c_bus_platform_data *pdata = pdev->dev.platform_data; + const struct omap_i2c_bus_platform_data *pdata = + pdev->dev.platform_data; struct device_node *node = pdev->dev.of_node; const struct of_device_id *match; irq_handler_t isr; -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 12/25] mmc/omap_hsmmc: add a const qualifier
This prepares *of_device_id.data becoming const. Without this change the following warning would occur: drivers/mmc/host/omap_hsmmc.c: In function 'omap_hsmmc_probe': drivers/mmc/host/omap_hsmmc.c:1808: warning: initialization discards qualifiers from pointer target type Signed-off-by: Uwe Kleine-König --- drivers/mmc/host/omap_hsmmc.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index 389a3ee..5bcd043 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -1805,7 +1805,7 @@ static int __devinit omap_hsmmc_probe(struct platform_device *pdev) if (match) { pdata = of_get_hsmmc_pdata(&pdev->dev); if (match->data) { - u16 *offsetp = match->data; + const u16 *offsetp = match->data; pdata->reg_offset = *offsetp; } } -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 02/25] spi/spi-omap2-mcspi: add a const qualifier
This prepares *of_device_id.data becoming const. Without this change the following warning would occur: drivers/spi/spi-omap2-mcspi.c: In function 'omap2_mcspi_probe': drivers/spi/spi-omap2-mcspi.c:1118: warning: assignment discards qualifiers from pointer target type Signed-off-by: Uwe Kleine-König --- drivers/spi/spi-omap2-mcspi.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/spi/spi-omap2-mcspi.c b/drivers/spi/spi-omap2-mcspi.c index 0c73dd4..3455769 100644 --- a/drivers/spi/spi-omap2-mcspi.c +++ b/drivers/spi/spi-omap2-mcspi.c @@ -1087,7 +1087,7 @@ MODULE_DEVICE_TABLE(of, omap_mcspi_of_match); static int __devinit omap2_mcspi_probe(struct platform_device *pdev) { struct spi_master *master; - struct omap2_mcspi_platform_config *pdata; + const struct omap2_mcspi_platform_config *pdata; struct omap2_mcspi *mcspi; struct resource *r; int status = 0, i; -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 19/25] of: add const to struct *of_device_id.data
Drivers should never need to modify the data of a device id. So it can be const which in turn allows more consts in the driver. Signed-off-by: Uwe Kleine-König --- include/linux/mod_devicetable.h |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h index 5db9382..98fb3fe 100644 --- a/include/linux/mod_devicetable.h +++ b/include/linux/mod_devicetable.h @@ -224,7 +224,7 @@ struct of_device_id chartype[32]; charcompatible[128]; #ifdef __KERNEL__ - void*data; + const void *data; #else kernel_ulong_t data; #endif -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 00/25] make *struct of_device_id.data const
Hello, this is the 2nd version of this series whose goal is to make struct of_device_id.data const. Conceptually a driver must not modify the data contained there so making it const is the right thing. v1 of this series was sent with Message-id: 1342182734-321-1-git-send-email-y. Changes since then are: - powerpc fixes - several new consts that were found by Arnd that are possible after patch 19. Arnd suggested to take this series via arm-soc late for 3.6 in one go because patch 19 depends on the former patches but is a precondition to the latter and it fixes a few warnings. So getting it in via the respective maintainer trees would need a much bigger coordination effort. That means I prefer getting Acks over you taking the patch. Vinod Koul already took dmaengine: at_hdmac: add a few const qualifiers that is in next-20120723 as 7fd63ccdad72 now. Vinod, I don't follow your pull requests, but assuming you didn't let it already pull for 3.6 I suggest you drop it from your queue and I just take your Ack. This series was build tested for arm (all defconfigs) and powerpc (all defconfigs and an allyesconfig) and grep didn't find more issues. As before it introduces a warning in drivers/regulator/twl-regulator.c. This driver does modify its .of_match_table when a device is bound which doesn't fits the concept of independant devices. Arnd noticed another new warning in drivers/scsi/qlogicpti.c that isn't that easy to resolve, because the pointer to (now) const data is passed as first argument to scsi_host_alloc. To fix that properly struct Scsi_Host.hostt needs to get a const, too. Alternatively I could introduce a cast removing the const, but I don't like that. This series is also available at: git://git.pengutronix.de/git/ukl/linux.git ofdeviceiddata and I will modify it there for the Acks I'm getting. Arnd Bergmann (6): watchdog/mpc8xxx: add a const qualifier powerpc/fsl_msi: drop unneeded cast to non-const pointer mfd/da9052: make i2c_device_id array const i2c/mpc: make data used as *of_device_id.data const macintosh/mediabay: make data used as *of_device_id.data const can: mpc5xxx_can: make data used as *of_device_id.data const Marc Kleine-Budde (1): can: mpc5xxx_can: make data in mpc5xxx_can_probe const Uwe Kleine-König (18): spi/imx: make spi_imx_data.devtype_data member point to const data spi/spi-omap2-mcspi: add a const qualifier serial/imx: make imx_port.devdata member point to const data serial/mpc52xx_uart: add a const qualifier ARM: cache-l2x0: add a const qualifier misc/atmel_tc: make atmel_tc.tcb_config member point to const data gpio/gpio-omap.c: add a const qualifier gpio/mpc8xxx: add a const qualifier i2c/i2c-omap: add a const qualifier i2c/mpc: add a const qualifier dmaengine: at_hdmac: add a few const qualifiers mmc/omap_hsmmc: add a const qualifier macintosh/mediabay: add a const qualifier powerpc/83xx: add a const qualifier powerpc/fsl_msi: add a const qualifier powerpc/celleb_pci: add a const qualifier of: add const to struct *of_device_id.data gpio/gpio-omap: make platformdata used as *of_device_id.data const arch/arm/mm/cache-l2x0.c |2 +- arch/powerpc/platforms/83xx/suspend.c|2 +- arch/powerpc/platforms/cell/celleb_pci.c |2 +- arch/powerpc/sysdev/fsl_msi.c|8 drivers/dma/at_hdmac.c |4 ++-- drivers/gpio/gpio-mpc8xxx.c |2 +- drivers/gpio/gpio-omap.c |8 drivers/i2c/busses/i2c-mpc.c | 12 ++-- drivers/i2c/busses/i2c-omap.c|3 ++- drivers/macintosh/mediabay.c |8 drivers/mfd/da9052-i2c.c |4 ++-- drivers/mmc/host/omap_hsmmc.c|2 +- drivers/net/can/mscan/mpc5xxx_can.c |6 +++--- drivers/spi/spi-imx.c|2 +- drivers/spi/spi-omap2-mcspi.c|2 +- drivers/tty/serial/imx.c |2 +- drivers/tty/serial/mpc52xx_uart.c|2 +- drivers/watchdog/mpc8xxx_wdt.c |2 +- include/linux/atmel_tc.h |2 +- include/linux/mod_devicetable.h |2 +- 20 files changed, 39 insertions(+), 38 deletions(-) -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/2] PWM: EHRPWM: PWM driver support for EHRPWM.
On Mon, Jul 23, 2012 at 13:12:21, Thierry Reding wrote: > On Fri, Jul 13, 2012 at 03:05:02PM +0530, Philip, Avinash wrote: > > Enhanced high resolution PWM module (EHRPWM) hardware can be used to > > generate PWM output over 2 channels. This commit adds PWM driver support > > for EHRPWM device present on AM33XX SOC. Current implementation supports > > simple PWM functionality. > > > > Signed-off-by: Philip, Avinash > > Reviewed-by: Vaibhav Bedia > > So this driver is very similar to the ECAP one and pretty much all the > comments apply to this as well. Some additional comments below. Ok I will correct it for all the comments. > > > --- [snip] > > + > > + To compile this driver as a module, choose M here: the module > > + will be called pwm-ehrpwm. > > + > > Maybe it would be useful to prefix the names with AM33XX here. ECAP and > EHRPWM are sort of generic and may have name clashes in the future. Ok, I will make us TI prefix as davinci platform also uses same modules. > > > +#define PWM_CHANNEL2 /* EHRPWM channels */ > > I'd say you can just replace the one occurrence of this with the literal > 2. If you still want to have the symbolic name, then I'd suggest to call > it something like NUM_PWM_CHANNELS to make its meaning more obvious. I will correct it as NUM_PWM_CHANNELS. > > > +static int __devexit ehrpwm_pwm_remove(struct platform_device *pdev) > > +{ > > + struct ehrpwm_pwm_chip *pc = platform_get_drvdata(pdev); > > + > > + if (WARN_ON(!pc)) > > + return -ENODEV; > > + > > + pm_runtime_disable(&pdev->dev); > > + pwmchip_remove(&pc->chip); > > + return 0; > > +} > > I forgot to mention this for ECAP, but you need to check the return > value of pwmchip_remove() because there are situations where it can > actually fail. I will correct it. Thanks Avinash > > Thierry > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] PWM: ECAP: PWM driver support for ECAP APWM.
On Mon, Jul 23, 2012 at 09:10:09AM +, Philip, Avinash wrote: > On Mon, Jul 23, 2012 at 12:22:35, Thierry Reding wrote: > > On Fri, Jul 13, 2012 at 03:05:01PM +0530, Philip, Avinash wrote: > > > + /* > > > + * Enable 'Free run Time stamp counter mode' to start counter > > > + * and 'APWM mode' to enable APWM output > > > + */ > > > + reg_val = readw(pc->mmio_base + ECCTL2); > > > + reg_val |= ECCTL2_TSCTR_FREERUN | ECCTL2_APWM_MODE; > > > > You already set the APWM_MODE bit in .config(), why is it needed here > > again? Seeing that .disable() clears the bit as well, perhaps leaving it > > clear in .config() is the better option. > > Clearing APWM mode in .disable() ensures PWM low output (i.e., PWM pin = low > in idle state). > > The Period & Compare registers are updated from Shadow registers (CAP1 & CAP2) > During PWM configuration. To enable loading from Shadow registers, APWM mode > should be set. > The same is done in .config(). My point was that if you do it in .enable(), then why do you still set it in .config()? Since the sequence is typically .config() followed by .enable(), setting the bit in both seems redundant. It should be enough to load the registers during .enable(), right? > > > + pc->chip.dev = &pdev->dev; > > > + pc->chip.ops = &ecap_pwm_ops; > > > + pc->chip.base = -1; > > > + pc->chip.npwm = 1; > > > > The cover letter mentions that the AM335x has 3 instances of the ECAP. > > Is there any chance that they can be unified in one driver instance > > (i.e. .npwm = 3?). > > No. All instances have separate resources (clocks, memory ..). > > > > > > + > > > + r = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ecap_reg"); > > > + if (r == NULL) { > > > > You use "if (!ptr)" everywhere else, maybe you should make this > > consistent? > Ok > > Also the platform_get_resource_byname() is really only > > useful for devices that have several register regions associated with > > them. I don't know your hardware in detail, but I doubt that a PWM > > device has more than one register region. > > In fact PWM Module has 4 different register space (eCAP, eHRPWM, eQEP & > Common Config space). So I need to use platform_get_resource_byname() Above you say that all instances have separate resources, so how come you need to specify 4 register spaces? The eCAP registers should clearly be passed to the eCAP device, while the eHRPWM registers should go to the eHRPWM device. My point is that if you need to refer to the register region by name, then the driver should clearly be using more than a single region. Neither the eCAP nor eHRPWM do that. Thierry pgp78hgwpgOrv.pgp Description: PGP signature
Re: [PATCH RESEND] usb: otg: OMAP4: Fix the omap4430_phy_set_clk function
Hi, On Thu, Jul 05, 2012 at 08:16:54PM +0300, Ruslan Bilovol wrote: > Hi, > > On Tue, Jul 3, 2012 at 7:13 PM, Felipe Balbi wrote: > > > > Hi, > > > > On Mon, Jul 02, 2012 at 08:10:49PM +0300, Ruslan Bilovol wrote: > > > Hi, > > > > > > > > > On Mon, Jul 2, 2012 at 11:13 AM, Felipe Balbi wrote: > > > > > > > > Hi, > > > > > > > > On Tue, Jun 12, 2012 at 08:36:21PM +0300, Ruslan Bilovol wrote: > > > > > If the clocks are enabled and we want to enable them again > > > > > omap4430_phy_set_clk disables them. > > > > > > > > > > Fix this - so now if we try to enable already enabled clocks > > > > > it works correctly. > > > > > > > > > > Signed-off-by: Ruslan Bilovol > > > > > --- > > > > > arch/arm/mach-omap2/omap_phy_internal.c |2 +- > > > > > 1 files changed, 1 insertions(+), 1 deletions(-) > > > > > > > > > > diff --git a/arch/arm/mach-omap2/omap_phy_internal.c > > > > > b/arch/arm/mach-omap2/omap_phy_internal.c > > > > > index 4c90477..0196683 100644 > > > > > --- a/arch/arm/mach-omap2/omap_phy_internal.c > > > > > +++ b/arch/arm/mach-omap2/omap_phy_internal.c > > > > > @@ -97,7 +97,7 @@ int omap4430_phy_set_clk(struct device *dev, int > > > > > on) > > > > > clk_enable(clk48m); > > > > > clk_enable(clk32k); > > > > > state = 1; > > > > > - } else if (state) { > > > > > + } else if (!on && state) { > > > > > > > > why don't you let clocks be enabled twice then ? That would cut down > > > > the > > > > churn. > > > > > > Currently we have unbalanced call of this function. > > > I meet first during musb initialization - it tries to disable the phy > > > that leads to disabling already disabled clocks. > > > Next goal is to use internal clocks counter and to throw static > > > variable 'state'. > > > > don't even go that way... what you need is to fix the unbalanced calls > > instead of hacking around some generic API. > > Okay Felipe, I understand your position and agree with you. However, > right now the 'hack' that I'm fixing works incorrectly. > So while we do not have replacement of the 'hack', it will be good to > have at least fixed version of this 'hack'. the problem is that once it "works" nobody ever looks into this again. So, sorry but I can't accept anything other than a real fix -- balbi signature.asc Description: Digital signature
bisected regression: arm: omap3: voltage: fix channel configuration
My GTA04 (mobile phone using OMAP3 and TWL4030 PMIC) has difficulty rebooting with 3.5. The first boot (after power on) works fine. But when I reboot it hangs just before Switched to new clocking rate (Crystal/Core/MPU): 26.0/332/800 MHz is printed. If I remove "mpurate=800" from the kernel command line the problem goes away. If I boot into 3.5, which works, and then reboot into an older working kernel it freezes too. So the 3.5 kernel leaves the device in a state which causes any kernel to hang. I spent a while bisecting and narrowed it down to commit f9d29f1617eb1b2f1fd41622bd1a0fc51658d2f0 Author: Tero Kristo Date: Mon Feb 20 12:26:06 2012 +0200 arm: omap3: voltage: fix channel configuration If I revert this patch then the problem goes away. So that is the fix I'll go with for now, but if anyone wants me to try something else to help find out what the real problem is and to find a proper fix, I'm happy to help. Thanks, NeilBrown signature.asc Description: PGP signature
Re: [PATCH] OMAP: USB : Fix the EHCI enumeration and core retention issue
On Mon, Jul 23, 2012 at 2:03 PM, Roger Quadros wrote: > Hi, > > On 06/22/2012 05:43 PM, Munegowda, Keshava wrote: >> On Fri, Jun 22, 2012 at 7:41 PM, Kevin Hilman wrote: >>> "Munegowda, Keshava" writes: >>> >>> [...] >>> > >>> >>> You are not reading what I write. >>> >>> To repeat: your patch fixes the oops during boot, and the suspend hang >>> and now I see CORE hit retention in *suspend*. >> >> thanks ! >> >>> >>> However, CORE does still not hit retention during *idle*. >> >> here is the problem. >> >> usb host retention in idle is not supported till now. >> in current code, usb host cuts clock only in driver suspend not in bus >> suspend ( auto suspend). >> usb host driver need to use the io daisy chain framework through io wakeup. >> I will post the patches once ehci remote wakeup features stabilized in >> omap3, omap4 and omap5 too. >> > > We are talking about CORE retention support during idle. How is IO daisy > chaining related to that? Doesn't IO daisy chain only apply when device > hits OFF? when we see the usb bus suspend, then we disable the clocks of usb host to enable to enable the retention in cpu idle; since we have disabled the clock of usb host , we will not see the device connection at the controller level, instead the irq chain handler can detect it and corresponding irq can set the clocks. this same use case holds good for device OFF too. regards keshava -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/2] PWM: ECAP: PWM driver support for ECAP APWM.
On Mon, Jul 23, 2012 at 14:52:04, Thierry Reding wrote: > On Mon, Jul 23, 2012 at 09:10:09AM +, Philip, Avinash wrote: > > On Mon, Jul 23, 2012 at 12:22:35, Thierry Reding wrote: > > > On Fri, Jul 13, 2012 at 03:05:01PM +0530, Philip, Avinash wrote: > > > > + /* > > > > +* Enable 'Free run Time stamp counter mode' to start counter > > > > +* and 'APWM mode' to enable APWM output > > > > +*/ > > > > + reg_val = readw(pc->mmio_base + ECCTL2); > > > > + reg_val |= ECCTL2_TSCTR_FREERUN | ECCTL2_APWM_MODE; > > > > > > You already set the APWM_MODE bit in .config(), why is it needed here > > > again? Seeing that .disable() clears the bit as well, perhaps leaving it > > > clear in .config() is the better option. > > > > Clearing APWM mode in .disable() ensures PWM low output (i.e., PWM pin = > > low > > in idle state). > > > > The Period & Compare registers are updated from Shadow registers (CAP1 & > > CAP2) > > During PWM configuration. To enable loading from Shadow registers, APWM > > mode > > should be set. > > The same is done in .config(). > > My point was that if you do it in .enable(), then why do you still set > it in .config()? Since the sequence is typically .config() followed by > .enable(), setting the bit in both seems redundant. It should be enough > to load the registers during .enable(), right? Consider scenarios where .enable() can be called without calling .config(). Example I just need to stop the pwm signal momentarily and re-enable. In this case, .config() need not be called. So, APWM mode bit needs to be set in .enable() Now, considering .config() followed by .enable(). Currently in PWM driver, the CAP1 & CAP2 registers is copied to shadow Registers (CAP3 & CAP4) by hardwaare in .config(). This requires APWM mode bit to be set. The same can be done in .enable() also. However, we again need to pass the pwm parameters (period & duty cycle values) to the .enable(). We don't want to duplicate this parameter passing. To avoid this we enable the APWM mode bit in both .config() & .enable(). > > > > > + pc->chip.dev = &pdev->dev; > > > > + pc->chip.ops = &ecap_pwm_ops; > > > > + pc->chip.base = -1; > > > > + pc->chip.npwm = 1; > > > > > > The cover letter mentions that the AM335x has 3 instances of the ECAP. > > > Is there any chance that they can be unified in one driver instance > > > (i.e. .npwm = 3?). > > > > No. All instances have separate resources (clocks, memory ..). > > > > > > > > > + > > > > + r = platform_get_resource_byname(pdev, IORESOURCE_MEM, > > > > "ecap_reg"); > > > > + if (r == NULL) { > > > > > > You use "if (!ptr)" everywhere else, maybe you should make this > > > consistent? > > Ok > > > Also the platform_get_resource_byname() is really only > > > useful for devices that have several register regions associated with > > > them. I don't know your hardware in detail, but I doubt that a PWM > > > device has more than one register region. > > > > In fact PWM Module has 4 different register space (eCAP, eHRPWM, eQEP & > > Common Config space). So I need to use platform_get_resource_byname() > > Above you say that all instances have separate resources, so how come > you need to specify 4 register spaces? The eCAP registers should clearly > be passed to the eCAP device, while the eHRPWM registers should go to > the eHRPWM device. > > My point is that if you need to refer to the register region by name, > then the driver should clearly be using more than a single region. > Neither the eCAP nor eHRPWM do that. On AM335x SoC, this common config space is shared by the other 3 modules (eCAP, eHRPWM, eQEP). However, on Davinci platform don't have any common config space. So from driver reusability view, usage of platform_get_resource_byname() is better choice. Thanks Avinash > > Thierry > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: OMAP: fix the ads7846 init code
Hi Kevin, On 07/12/12 02:00, Kevin Hilman wrote: > Hi Igor, > > Igor Grinberg writes: > >> In case a board provides the gpio_pendown and not board_pdata, >> the GPIO debounce is not taken care of. >> Fix this by taking care of GPIO debounce in any case. >> >> Signed-off-by: Igor Grinberg > > I just notice this this patch causing some faults in current l-o master > branch. This was not the case by the time I did the patch... > >> --- >> arch/arm/mach-omap2/common-board-devices.c | 22 -- >> 1 files changed, 12 insertions(+), 10 deletions(-) >> >> diff --git a/arch/arm/mach-omap2/common-board-devices.c >> b/arch/arm/mach-omap2/common-board-devices.c >> index 1706ebc..c187586 100644 >> --- a/arch/arm/mach-omap2/common-board-devices.c >> +++ b/arch/arm/mach-omap2/common-board-devices.c >> @@ -63,28 +63,30 @@ void __init omap_ads7846_init(int bus_num, int >> gpio_pendown, int gpio_debounce, >> struct spi_board_info *spi_bi = &ads7846_spi_board_info; >> int err; >> >> -if (board_pdata && board_pdata->get_pendown_state) { >> -err = gpio_request_one(gpio_pendown, GPIOF_IN, "TSPenDown"); >> -if (err) { >> -pr_err("Couldn't obtain gpio for TSPenDown: %d\n", err); >> -return; >> -} >> -gpio_export(gpio_pendown, 0); >> - >> -if (gpio_debounce) >> -gpio_set_debounce(gpio_pendown, gpio_debounce); >> +err = gpio_request_one(gpio_pendown, GPIOF_IN, "TSPenDown"); >> +if (err) { >> +pr_err("Couldn't obtain gpio for TSPenDown: %d\n", err); >> +return; >> } >> >> +if (gpio_debounce) >> +gpio_set_debounce(gpio_pendown, gpio_debounce); >> + >> spi_bi->bus_num = bus_num; >> spi_bi->irq = gpio_to_irq(gpio_pendown); >> >> if (board_pdata) { >> board_pdata->gpio_pendown = gpio_pendown; >> spi_bi->platform_data = board_pdata; >> +if (board_pdata->get_pendown_state) >> +gpio_export(gpio_pendown, 0); >> } else { >> ads7846_config.gpio_pendown = gpio_pendown; >> } >> >> +if (!board_pdata || (board_pdata && !board_pdata->get_pendown_state)) >> +gpio_free(gpio_pendown); > > The logic here for freeing the GPIO doesn't make any sense to me. > IIUC, the gpio_pendown is always used, so should not be freed. The gpio_pendown is requested by the ads7846 driver and of course you can't request the GPIO second time, so the driver failed. > > Moreover, if this GPIO is freed, that allows that GPIO bank to runtime > suspend if there are no other GPIOs in use in that bank. So the first > attempt to use the GPIO will fault. The ads7846 driver requests the GPIO prior to using it. > > For example, on my Overo board(s), I noticed this failing as soon as teh > ads7846 driver probes, requests the IRQ and the GPIO triggering is set. Prior to requesting the IRQ, the ads7846_setup_pendown(spi, ts) method is called, which requests the GPIO. > > Getting rid of this free fixes the problem. But then don't you have double requesting? > > The changelog doesn't describe why the GPIO is freed here so I'm not > sure I follow, but it seems to me that once this GPIO is requesed, it > should not be freed as long as it's used, otherwise PM faults can occur. It is not used until the ads7846 driver probes. > > Kevin > >>From bb87c3b5586950e480d0699504997a9ad587fd85 Mon Sep 17 00:00:00 2001 > From: Kevin Hilman > Date: Wed, 11 Jul 2012 15:47:29 -0700 > Subject: [PATCH] ARM: OMAP2+: ads7846 init: fix fault caused by freeing > pen-down GPIO > > commit 97ee9f01d6 (ARM: OMAP: fix the ads7846 init code) mistakenly > frees the pen-down GPIO even though it will be used by the ads7846 > driver. > > Freeing a GPIO means that the GPIO bank containing that GPIO can be > runtime suspended if its the last/only GPIO being used in that bank. > If the GPIO bank is runtime suspended, any accesses to that bank will > cause faults. > > Because the current code frees the GPIO, the ads7846 driver probe will > fault when it requests its IRQ line. Because the IRQ is a GPIO line, > the request IRQ will trickle down into the OMAP GPIO layer: > gpio_irq_type() --> _set_gpio_triggering() which can fault if the > bank has been runtime suspended. > > This is exctly what happens on Overo platforms (3530 Water, 3730 Overo > FireSTORM) since this is the only GPIO used in the bank. > > To fix, don't free the GPIO at all since it is always in use. > > Cc: Igor Grinberg > Signed-off-by: Kevin Hilman > --- > arch/arm/mach-omap2/common-board-devices.c |3 --- > 1 file changed, 3 deletions(-) > > diff --git a/arch/arm/mach-omap2/common-board-devices.c > b/arch/arm/mach-omap2/common-board-devices.c > index c187586..1ae6fd6 100644 > --- a/arch/arm/mach-omap2/common-board-devices.c > +++ b/arch/arm/mach-omap2/common-board-devices.c > @@ -84
RE: [PATCH] PWM: Add support for configuring polarity of PWM
On Mon, Jul 23, 2012 at 14:00:32, Thierry Reding wrote: > On Wed, Jul 18, 2012 at 06:24:13PM +0530, Philip, Avinash wrote: > > Duty cycle inversion of PWM wave should achieved through PWM polarity > > inversion. Also polarity of PWM wave should configurable from slave > > drivers, > > Actually, I don't think that duty cycle inversion *should* be achieved > through polarity inversion. But it is true that the same effect *can* be > achieved by inverting the polarity. Correct. > > > Configure polarity > > 1. PWM_POLARITY_NORMAL -> duty ns defines ON period of PWM wave > > 2. PWM_POLARITY_INVERSE -> duty ns defines OFF period of PWM wave. > > Similarly, this text describes how polarity inversion can be used to > simular duty cycle inversion. > > I think you should describe that this change adds support for > configuring the PWM polarity. If you absolutely must note that it can be > used to simulate the duty cycle inversion, then you can give it as an > example below the actual description. > > > This patch adds support for configuring PWM polarity in PWM frame work. > > "framework" is one word. Ok. > > > > > Signed-off-by: Philip, Avinash > > --- > > Configuring polarity to be done with PWM device disabled, if not > > failure reported. > > > > If PWM device is enabled while configuring polarity, disabling and > > re_enabling make it complex. Whoever uses this API has to taken > > care of the basic rules. > > These comments belong in the commit message because they are very useful > information about the change that you introduce. They probably belong > somewhere in the code as well. > > > Discussions related to this can found at > > http://www.spinics.net/lists/kernel/msg1372110.html > > Here's my proposal for a revised commit message: > > pwm: Add support for configuring the PWM polarity > > Some hardware supports inverting the polarity of the PWM signal. This > commit adds support to the PWM framework to allow users of the PWM API > to configure the polarity. Note that in order to reduce complexity, > changing the polarity of a PWM signal is only allowed while the PWM is > disabled. > > A practical example where this can prove useful is to simulate inversion > of the duty cycle. While inversion of polarity and duty cycle are not > exactly the same, the differences for most use-cases are negligible. Ok I will update. > > > :100644 100644 ecb7690... 24d5495... M drivers/pwm/core.c > > :100644 100644 21d076c... 2e4e960... M include/linux/pwm.h > > drivers/pwm/core.c | 32 > > include/linux/pwm.h | 15 +++ > > 2 files changed, 47 insertions(+), 0 deletions(-) > > > > diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c > > index ecb7690..24d5495 100644 > > --- a/drivers/pwm/core.c > > +++ b/drivers/pwm/core.c > > @@ -379,6 +379,38 @@ int pwm_config(struct pwm_device *pwm, int duty_ns, > > int period_ns) > > EXPORT_SYMBOL_GPL(pwm_config); > > > > /** > > + * pwm_set_polarity() - change PWM device Polarity > > Maybe: "pwm_set_polarity() - configure the polarity of a PWM signal" > > > + * @pwm: PWM device > > + * @polarity: Configure polarity of PWM > > "new polarity of the PWM signal" > > > + * > > + * Polarity to be configured with PWM device disabled. > > "Note that the polarity cannot be configured while the PWM device is > enabled." Ok I will update. > > > + */ > > +int pwm_set_polarity(struct pwm_device *pwm, int polarity) > > The polarity parameter should be an enum pwm_polarity, see below. > > > +{ > > + int pwm_flags = PWM_POLARITY_NORMAL; > > I don't think this is needed. > > > + > > + if (!pwm || !pwm->chip->ops->set_polarity) > > + return -EINVAL; > > I'd prefer -ENOSYS if .set_polarity is not implemented, so this check > should probably be split up: > > if (!pwm || !pwm->chip || !pwm->chip->ops) > return -EINVAL; > > if (!pwm->chip->ops->set_polarity) > return -ENOSYS; I will correct it. > > > + > > + if (test_bit(PWMF_ENABLED, &pwm->flags)) { > > + dev_err(pwm->chip->dev, > > + "Polarity configuration Failed!, PWM device enabled\n"); > > + return -EBUSY; > > + } > > Maybe something like: "polarity cannot be configured while PWM device is > enabled"? Ok I will update. > Though I'm not sure the error message is all that useful. I'd > expect the user driver to handle -EBUSY specially. On EBUSY, client driver has to rework on it. Nothing to be done from framework > > > + > > + if (polarity == PWM_POLARITY_INVERSE) > > + pwm_flags = PWM_POLARITY_INVERSE; > > + > > + if (!pwm_flags) > > + clear_bit(PWMF_POLARITY_INVERSE, &pwm->flags); > > + else > > + set_bit(PWMF_POLARITY_INVERSE, &pwm->flags); > > You can make this decision based on the value of polarity, no need for > the additional variable. Also as I
Re: [PATCH] ARM: OMAP2+: ads7846 init: fix fault caused by freeing pen-down GPIO
On 07/12/12 02:18, Kevin Hilman wrote: > commit 97ee9f01d6 (ARM: OMAP: fix the ads7846 init code) mistakenly > frees the pen-down GPIO even though it will be used by the ads7846 > driver. But the driver requests the GPIO in ads7846_setup_pendown() method... > > Freeing a GPIO means that the GPIO bank containing that GPIO can be > runtime suspended if its the last/only GPIO being used in that bank. > If the GPIO bank is runtime suspended, any accesses to that bank will > cause faults. Unless, it is requested again, or am I missing something? > > Because the current code frees the GPIO, the ads7846 driver probe will > fault when it requests its IRQ line. But it requests that GPIO... in ads7846_setup_pendown() prior to requesting the IRQ... > Because the IRQ is a GPIO line, > the request IRQ will trickle down into the OMAP GPIO layer: > gpio_irq_type() --> _set_gpio_triggering() which can fault if the > bank has been runtime suspended. > > This is exctly what happens on Overo platforms (3530 Water, 3730 Overo > FireSTORM) since this is the only GPIO used in the bank. > > To fix, don't free the GPIO at all since it is always in use. > > Cc: Igor Grinberg > Signed-off-by: Kevin Hilman > --- > Tony, this applies on top of your current fixes-non-critical branch and > should probably go in to v3.5-rc since the patch which introduced the > problem did as well. > > arch/arm/mach-omap2/common-board-devices.c |3 --- > 1 file changed, 3 deletions(-) > > diff --git a/arch/arm/mach-omap2/common-board-devices.c > b/arch/arm/mach-omap2/common-board-devices.c > index c187586..1ae6fd6 100644 > --- a/arch/arm/mach-omap2/common-board-devices.c > +++ b/arch/arm/mach-omap2/common-board-devices.c > @@ -84,9 +84,6 @@ void __init omap_ads7846_init(int bus_num, int > gpio_pendown, int gpio_debounce, > ads7846_config.gpio_pendown = gpio_pendown; > } > > - if (!board_pdata || (board_pdata && !board_pdata->get_pendown_state)) > - gpio_free(gpio_pendown); > - > spi_register_board_info(&ads7846_spi_board_info, 1); > } > #else -- Regards, Igor. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PWM: Add support for configuring polarity of PWM
On Mon, Jul 23, 2012 at 12:51:11PM +, Philip, Avinash wrote: > On Mon, Jul 23, 2012 at 14:00:32, Thierry Reding wrote: > > On Wed, Jul 18, 2012 at 06:24:13PM +0530, Philip, Avinash wrote: [...] > > > + > > > + if (test_bit(PWMF_ENABLED, &pwm->flags)) { > > > + dev_err(pwm->chip->dev, > > > + "Polarity configuration Failed!, PWM device enabled\n"); > > > + return -EBUSY; > > > + } > > > > Maybe something like: "polarity cannot be configured while PWM device is > > enabled"? > > Ok I will update. > > > Though I'm not sure the error message is all that useful. I'd > > expect the user driver to handle -EBUSY specially. > > On EBUSY, client driver has to rework on it. Nothing to be done from > framework Exactly, so I think that if an error is displayed because the PWM has been enabled, then that client (== user) driver should output an error message, not the framework. Also, it really shouldn't happen because it clearly is a driver problem that needs to be fixed. > > > /* > > > * pwm_enable - start a PWM output toggling > > > */ > > > @@ -37,6 +47,7 @@ struct pwm_chip; > > > enum { > > > PWMF_REQUESTED = 1 << 0, > > > PWMF_ENABLED = 1 << 1, > > > + PWMF_POLARITY_INVERSE = 1 << 2, > > > > This should be named PWMF_POLARITY_INVERSED for consistency. > > Ok I will correct it. > > > I'm not sure that we really need this flag, though. It isn't used anywhere. > > But > > maybe you have a use-case in mind? > > It can be used to find the polarity of the PWM at runtime. Yes, but is there any use-case where this information would be required? Thierry pgpoKjMJYdr8Z.pgp Description: PGP signature
Re: [PATCH] ARM: OMAP2+: ads7846 init: fix fault caused by freeing pen-down GPIO
Hi Kevin, I've just noticed that the patch has been modified by Arnd in a way that of course will trigger GPIO use without being requested. I'm sorry, I was not available by that time Arnd changed the patch. So that is not my original patch that is triggering the issue, but it does not meter already... And currently, I think that the proper solution would be to remove the gpio_free() call as your patch does, but... Tony, please pay attention to which branch it gets applied It should only apply to the branches that have the patch modified by Arnd and not to those with unmodified patch (e.g. fixes-non-critical should not receive the patch). Kevin, thanks for the patch. On 07/23/12 15:53, Igor Grinberg wrote: > On 07/12/12 02:18, Kevin Hilman wrote: >> commit 97ee9f01d6 (ARM: OMAP: fix the ads7846 init code) mistakenly >> frees the pen-down GPIO even though it will be used by the ads7846 >> driver. > > But the driver requests the GPIO in ads7846_setup_pendown() method... > >> >> Freeing a GPIO means that the GPIO bank containing that GPIO can be >> runtime suspended if its the last/only GPIO being used in that bank. >> If the GPIO bank is runtime suspended, any accesses to that bank will >> cause faults. > > Unless, it is requested again, or am I missing something? > >> >> Because the current code frees the GPIO, the ads7846 driver probe will >> fault when it requests its IRQ line. > > But it requests that GPIO... in ads7846_setup_pendown() > prior to requesting the IRQ... > >> Because the IRQ is a GPIO line, >> the request IRQ will trickle down into the OMAP GPIO layer: >> gpio_irq_type() --> _set_gpio_triggering() which can fault if the >> bank has been runtime suspended. >> >> This is exctly what happens on Overo platforms (3530 Water, 3730 Overo >> FireSTORM) since this is the only GPIO used in the bank. >> >> To fix, don't free the GPIO at all since it is always in use. >> >> Cc: Igor Grinberg >> Signed-off-by: Kevin Hilman Acked-by: Igor Grinberg >> --- >> Tony, this applies on top of your current fixes-non-critical branch and >> should probably go in to v3.5-rc since the patch which introduced the >> problem did as well. No, this patch is not good for fixes-non-critical, as fixes-non-critical has unmodified version and will result in double requesting. It should be applied to master or derivatives that have the modified patch version. >> >> arch/arm/mach-omap2/common-board-devices.c |3 --- >> 1 file changed, 3 deletions(-) >> >> diff --git a/arch/arm/mach-omap2/common-board-devices.c >> b/arch/arm/mach-omap2/common-board-devices.c >> index c187586..1ae6fd6 100644 >> --- a/arch/arm/mach-omap2/common-board-devices.c >> +++ b/arch/arm/mach-omap2/common-board-devices.c >> @@ -84,9 +84,6 @@ void __init omap_ads7846_init(int bus_num, int >> gpio_pendown, int gpio_debounce, >> ads7846_config.gpio_pendown = gpio_pendown; >> } >> >> -if (!board_pdata || (board_pdata && !board_pdata->get_pendown_state)) >> -gpio_free(gpio_pendown); >> - >> spi_register_board_info(&ads7846_spi_board_info, 1); >> } >> #else > -- Regards, Igor. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH RESEND 1/4] ARM: OMAP2+: AM33XX: Add tps65910 device tree data
On Mon, Jul 23, 2012 at 12:36:48, AnilKumar, Chimata wrote: > On Fri, Jul 20, 2012 at 17:08:06, Mark Brown wrote: > > On Fri, Jul 20, 2012 at 11:27:36AM +, AnilKumar, Chimata wrote: > > > On Fri, Jul 20, 2012 at 15:29:36, Mark Brown wrote: > > > > > > Every regulator here has a rather large voltage range specified with no > > > > consumers added. Are you sure these voltage ranges make sense in your > > > > design and you've not just cut'n'pasted the entire voltage range that > > > > your regulator supports without reference to what your board can do? > > > > > tps65217.dtsi is a generic file to be used by the SoCs so these > > > constraints > > > were taken from the regulator itself. SoC specific limits can be added in > > > SoC specific .dts file to tighten the constraints to require limit. I have > > > tested the driver with this approach. > > > > No, this is not a sane approach. You've no idea if any of these > > settings are safe or sane for the board. Boards should enable things > > they know are safe, not remove those they know are broken. > > > > Unsterstood, I will send v2 with constraints updated. > By the way, if we look at all the regulator added (DT supported) till now have the similar problem. arch/arm/boot/dts/imx6q.dtsi arch/arm/boot/dts/twl4030.dtsi arch/arm/boot/dts/twl6030.dtsi arch/arm/boot/dts/db8500.dtsi Regards AnilKumar -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RESEND 1/4] ARM: OMAP2+: AM33XX: Add tps65910 device tree data
On Mon, Jul 23, 2012 at 01:23:50PM +, AnilKumar, Chimata wrote: > By the way, if we look at all the regulator added (DT supported) till now have > the similar problem. > arch/arm/boot/dts/imx6q.dtsi This is fine - the SoC contains integrated regulators which supply other bits of the Soc so we can be confident that the hookup is good just based on the silicon. > arch/arm/boot/dts/twl4030.dtsi > arch/arm/boot/dts/twl6030.dtsi These appear to have similar issues and should be fixed, at least as far as the voltage ranges go. > arch/arm/boot/dts/db8500.dtsi I'm not actually seeing anything terribly problematic here, though the regulator-name properties should really be removed as they're fairly useless and seem to be missing the point of having the property. signature.asc Description: Digital signature
Re: [PATCH 1/2] PWM: ECAP: PWM driver support for ECAP APWM.
On Mon, Jul 23, 2012 at 12:44:38PM +, Philip, Avinash wrote: > On Mon, Jul 23, 2012 at 14:52:04, Thierry Reding wrote: > > On Mon, Jul 23, 2012 at 09:10:09AM +, Philip, Avinash wrote: > > > On Mon, Jul 23, 2012 at 12:22:35, Thierry Reding wrote: > > > > On Fri, Jul 13, 2012 at 03:05:01PM +0530, Philip, Avinash wrote: > > > > > + /* > > > > > + * Enable 'Free run Time stamp counter mode' to start counter > > > > > + * and 'APWM mode' to enable APWM output > > > > > + */ > > > > > + reg_val = readw(pc->mmio_base + ECCTL2); > > > > > + reg_val |= ECCTL2_TSCTR_FREERUN | ECCTL2_APWM_MODE; > > > > > > > > You already set the APWM_MODE bit in .config(), why is it needed here > > > > again? Seeing that .disable() clears the bit as well, perhaps leaving it > > > > clear in .config() is the better option. > > > > > > Clearing APWM mode in .disable() ensures PWM low output (i.e., PWM pin = > > > low > > > in idle state). > > > > > > The Period & Compare registers are updated from Shadow registers (CAP1 & > > > CAP2) > > > During PWM configuration. To enable loading from Shadow registers, APWM > > > mode > > > should be set. > > > The same is done in .config(). > > > > My point was that if you do it in .enable(), then why do you still set > > it in .config()? Since the sequence is typically .config() followed by > > .enable(), setting the bit in both seems redundant. It should be enough > > to load the registers during .enable(), right? > > Consider scenarios where .enable() can be called without calling .config(). > Example I just need to stop the pwm signal momentarily and re-enable. > In this case, .config() need not be called. So, APWM mode bit needs to be > set in .enable() > > Now, considering .config() followed by .enable(). > Currently in PWM driver, the CAP1 & CAP2 registers is copied to shadow > Registers (CAP3 & CAP4) by hardwaare in .config(). This requires APWM > mode bit to be set. > > The same can be done in .enable() also. However, we again need to pass > the pwm parameters (period & duty cycle values) to the .enable(). > We don't want to duplicate this parameter passing. > To avoid this we enable the APWM mode bit in both .config() & .enable(). That's weird. I assumed that the values written to the shadow registers would automatically be copied to the active registers on each new PWM period. If I understand correctly what you're saying, however, the eCAP requires the APWM bit to be set in order to write the shadow registers at all. > > > > > + pc->chip.dev = &pdev->dev; > > > > > + pc->chip.ops = &ecap_pwm_ops; > > > > > + pc->chip.base = -1; > > > > > + pc->chip.npwm = 1; > > > > > > > > The cover letter mentions that the AM335x has 3 instances of the ECAP. > > > > Is there any chance that they can be unified in one driver instance > > > > (i.e. .npwm = 3?). > > > > > > No. All instances have separate resources (clocks, memory ..). > > > > > > > > > > > > + > > > > > + r = platform_get_resource_byname(pdev, IORESOURCE_MEM, > > > > > "ecap_reg"); > > > > > + if (r == NULL) { > > > > > > > > You use "if (!ptr)" everywhere else, maybe you should make this > > > > consistent? > > > Ok > > > > Also the platform_get_resource_byname() is really only > > > > useful for devices that have several register regions associated with > > > > them. I don't know your hardware in detail, but I doubt that a PWM > > > > device has more than one register region. > > > > > > In fact PWM Module has 4 different register space (eCAP, eHRPWM, eQEP & > > > Common Config space). So I need to use platform_get_resource_byname() > > > > Above you say that all instances have separate resources, so how come > > you need to specify 4 register spaces? The eCAP registers should clearly > > be passed to the eCAP device, while the eHRPWM registers should go to > > the eHRPWM device. > > > > My point is that if you need to refer to the register region by name, > > then the driver should clearly be using more than a single region. > > Neither the eCAP nor eHRPWM do that. > > On AM335x SoC, this common config space is shared by the other 3 > modules (eCAP, eHRPWM, eQEP). > However, on Davinci platform don't have any common config space. > > So from driver reusability view, usage of platform_get_resource_byname() > is better choice. I don't quite see how you would be able to reuse the driver in that case. The driver that you posted uses only one memory region, so the platform device that you instantiate for the driver to bind to only gets the one region through the .resource and .num_resources fields. How is that going to work? Thierry pgp75iwZsul1D.pgp Description: PGP signature
omap: omap_mux_init_gpio() for gpio_0
I've i2c gpio expander (pca953x) IRQ attached to pin SYS_NIRQ/GPIO_0 (am3517). How can I declare this pin as GPIO and later as IRQ? if (!gpio) return -EINVAL; list_for_each_entry(e, muxmodes, node) { struct omap_mux *m = &e->mux; if (gpio == m->gpio) { gpio_mux = m; found++; } } if (found == 0) { pr_err("%s: Could not set gpio%i\n", __func__, gpio); return -ENODEV; } if (found > 1) { pr_info("%s: Multiple gpio paths (%d) for gpio%i\n", __func__, found, gpio); return -EINVAL; } with this semantic in arch/arm/mach-omap2/mux.c it is not possible and if I comment the first constraint I get "Multiple gpio paths" error, because found == 5. Yegor -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V4 0/5] ARM: OMAP: HOST: TLL driver implementation
On Thu, Jul 19, 2012 at 3:48 PM, Munegowda, Keshava wrote: > On Mon, Jul 16, 2012 at 7:01 PM, Keshava Munegowda > wrote: >> The TLL (Transceiver Less Link) is an separate IP block, independent of >> the host controller. Basically this TLL operates like USB PHY which allows >> the user to connect two USB transceiver interfaces together directly >> without the use of differential transceivers in omap3 and later chips. >> The TLL configuration is removed from the UHH driver and implemented as >> a seperate platform driver. Now, the UHH driver configures the TLL >> through API's exported by the TLL platform driver. >> >> Signed-off-by: Keshava Munegowda >> >> In v4: >> - rebased on top of linux kernel version 3.5.rc7 >> - reworked as per the comments given by Paul Walmsley >> >> In v3: >> - rebased on top V3 of Russ dill's patch >>'ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset issue' >>fixes an issue where the ULPI PHYs were not held in reset >>while initializing the EHCI controller >> http://permalink.gmane.org/gmane.linux.usb.general/65988 >> >> - rebased on top of patch >> OMAP: USB : Fix the EHCI enumeration and core retention issue >> http://permalink.gmane.org/gmane.linux.usb.general/66239 >> >> In V2: >> - covered review comments from linux omap and usb community >> - rebased on top Russ dill's patch >>'ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset issue' >>fixes an issue where the ULPI PHYs were not held in reset >>while initializing the EHCI controller >> >> Keshava Munegowda (5): >> ARM: OMAP: Add the USB TLL clocks device name >> ARM: OMAP: USB: HOST TLL platform driver >> ARM: OMAP: USB: Invoke the TLL driver from USB HS core driver >> ARM: OMAP: USB: Remove TLL specific code from USB HS core driver >> ARM: OMAP: Remove older device name of the USB TLL clocks >> >> arch/arm/mach-omap2/clock3xxx_data.c |8 +- >> arch/arm/mach-omap2/clock44xx_data.c |4 +- >> arch/arm/mach-omap2/usb-host.c| 31 ++- >> arch/arm/plat-omap/include/plat/usb.h |7 + >> drivers/mfd/Kconfig |2 +- >> drivers/mfd/Makefile |2 +- >> drivers/mfd/omap-usb-host.c | 238 ++--- >> drivers/mfd/omap-usb-tll.c| 471 >> + >> 8 files changed, 523 insertions(+), 240 deletions(-) >> create mode 100644 drivers/mfd/omap-usb-tll.c >> >> -- >> 1.7.9.5 > > > Felipe/ Paul > please let me know if you have any review comments on this v4 series. > > regards > keshava Hi Felipe please let me know if you have any review comments on this series now. regards keshava -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V4 0/5] ARM: OMAP: HOST: TLL driver implementation
Hi, On Mon, Jul 23, 2012 at 07:31:10PM +0530, Munegowda, Keshava wrote: > On Thu, Jul 19, 2012 at 3:48 PM, Munegowda, Keshava > wrote: > > On Mon, Jul 16, 2012 at 7:01 PM, Keshava Munegowda > > wrote: > >> The TLL (Transceiver Less Link) is an separate IP block, independent of > >> the host controller. Basically this TLL operates like USB PHY which allows > >> the user to connect two USB transceiver interfaces together directly > >> without the use of differential transceivers in omap3 and later chips. > >> The TLL configuration is removed from the UHH driver and implemented as > >> a seperate platform driver. Now, the UHH driver configures the TLL > >> through API's exported by the TLL platform driver. > >> > >> Signed-off-by: Keshava Munegowda > >> > >> In v4: > >> - rebased on top of linux kernel version 3.5.rc7 > >> - reworked as per the comments given by Paul Walmsley > >> > >> In v3: > >> - rebased on top V3 of Russ dill's patch > >>'ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset issue' > >>fixes an issue where the ULPI PHYs were not held in reset > >>while initializing the EHCI controller > >> http://permalink.gmane.org/gmane.linux.usb.general/65988 > >> > >> - rebased on top of patch > >> OMAP: USB : Fix the EHCI enumeration and core retention issue > >> http://permalink.gmane.org/gmane.linux.usb.general/66239 > >> > >> In V2: > >> - covered review comments from linux omap and usb community > >> - rebased on top Russ dill's patch > >>'ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset issue' > >>fixes an issue where the ULPI PHYs were not held in reset > >>while initializing the EHCI controller > >> > >> Keshava Munegowda (5): > >> ARM: OMAP: Add the USB TLL clocks device name > >> ARM: OMAP: USB: HOST TLL platform driver > >> ARM: OMAP: USB: Invoke the TLL driver from USB HS core driver > >> ARM: OMAP: USB: Remove TLL specific code from USB HS core driver > >> ARM: OMAP: Remove older device name of the USB TLL clocks > >> > >> arch/arm/mach-omap2/clock3xxx_data.c |8 +- > >> arch/arm/mach-omap2/clock44xx_data.c |4 +- > >> arch/arm/mach-omap2/usb-host.c| 31 ++- > >> arch/arm/plat-omap/include/plat/usb.h |7 + > >> drivers/mfd/Kconfig |2 +- > >> drivers/mfd/Makefile |2 +- > >> drivers/mfd/omap-usb-host.c | 238 ++--- > >> drivers/mfd/omap-usb-tll.c| 471 > >> + > >> 8 files changed, 523 insertions(+), 240 deletions(-) > >> create mode 100644 drivers/mfd/omap-usb-tll.c > >> > >> -- > >> 1.7.9.5 > > > > > > Felipe/ Paul > > please let me know if you have any review comments on this v4 series. > > > > regards > > keshava > > Hi Felipe > please let me know if you have any review comments on this series > now. looks ok... pretty much just moving code around. -- balbi signature.asc Description: Digital signature
Re: bisected regression: arm: omap3: voltage: fix channel configuration
Hi Neil, On Mon, 2012-07-23 at 21:06 +1000, NeilBrown wrote: > My GTA04 (mobile phone using OMAP3 and TWL4030 PMIC) has difficulty rebooting > with 3.5. > The first boot (after power on) works fine. But when I reboot it hangs just > before > > Switched to new clocking rate (Crystal/Core/MPU): 26.0/332/800 MHz > > is printed. If I remove "mpurate=800" from the kernel command line the > problem goes away. > > If I boot into 3.5, which works, and then reboot into an older working kernel > it freezes too. So the 3.5 kernel leaves the device in a state which causes > any kernel to hang. TWL chip retains its settings over warm boot which most likely explains this. > > I spent a while bisecting and narrowed it down to > > commit f9d29f1617eb1b2f1fd41622bd1a0fc51658d2f0 > Author: Tero Kristo > Date: Mon Feb 20 12:26:06 2012 +0200 > > arm: omap3: voltage: fix channel configuration > > > If I revert this patch then the problem goes away. So that is the fix I'll > go with for now, but if anyone wants me to try something else to help find > out what the real problem is and to find a proper fix, I'm happy to help. Do you have cpufreq enabled in your build? If yes, then most likely the crash occurs because cpufreq scales the used OPP to minimum after boot and the warm reset doesn't switch the voltages properly and crashes as it attempts to raise CPU frequency. Also, if cpufreq is enabled, mpurate command line option doesn't really do much... it will just switch the frequency to the desired target during beginning of boot until cpufreq kicks in and switches the OPP to its own understanding of desired cpu rate. I think the handling of mpurate command line option should probably be ignored in cpufreq builds, or moved within the cpufreq driver. I tried to craft a quick boot time fix for this problem, but it seems the voltage layer is not properly initialized yet when the mpurate setting kicks in and it is not very straightforward to scale voltage during this time without writing a serious hack. If you don't have cpufreq enabled, then the vdd routing on your board might be interesting (I don't think this is the case as the kernel works okay after 1st boot.) -Tero -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V4 0/5] ARM: OMAP: HOST: TLL driver implementation
On Mon, Jul 23, 2012 at 7:38 PM, Felipe Balbi wrote: > Hi, > > On Mon, Jul 23, 2012 at 07:31:10PM +0530, Munegowda, Keshava wrote: >> On Thu, Jul 19, 2012 at 3:48 PM, Munegowda, Keshava >> wrote: >> > On Mon, Jul 16, 2012 at 7:01 PM, Keshava Munegowda >> > wrote: >> >> The TLL (Transceiver Less Link) is an separate IP block, independent of >> >> the host controller. Basically this TLL operates like USB PHY which allows >> >> the user to connect two USB transceiver interfaces together directly >> >> without the use of differential transceivers in omap3 and later chips. >> >> The TLL configuration is removed from the UHH driver and implemented as >> >> a seperate platform driver. Now, the UHH driver configures the TLL >> >> through API's exported by the TLL platform driver. >> >> >> >> Signed-off-by: Keshava Munegowda >> >> >> >> In v4: >> >> - rebased on top of linux kernel version 3.5.rc7 >> >> - reworked as per the comments given by Paul Walmsley >> >> >> >> In v3: >> >> - rebased on top V3 of Russ dill's patch >> >>'ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset issue' >> >>fixes an issue where the ULPI PHYs were not held in reset >> >>while initializing the EHCI controller >> >> http://permalink.gmane.org/gmane.linux.usb.general/65988 >> >> >> >> - rebased on top of patch >> >> OMAP: USB : Fix the EHCI enumeration and core retention issue >> >> http://permalink.gmane.org/gmane.linux.usb.general/66239 >> >> >> >> In V2: >> >> - covered review comments from linux omap and usb community >> >> - rebased on top Russ dill's patch >> >>'ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset issue' >> >>fixes an issue where the ULPI PHYs were not held in reset >> >>while initializing the EHCI controller >> >> >> >> Keshava Munegowda (5): >> >> ARM: OMAP: Add the USB TLL clocks device name >> >> ARM: OMAP: USB: HOST TLL platform driver >> >> ARM: OMAP: USB: Invoke the TLL driver from USB HS core driver >> >> ARM: OMAP: USB: Remove TLL specific code from USB HS core driver >> >> ARM: OMAP: Remove older device name of the USB TLL clocks >> >> >> >> arch/arm/mach-omap2/clock3xxx_data.c |8 +- >> >> arch/arm/mach-omap2/clock44xx_data.c |4 +- >> >> arch/arm/mach-omap2/usb-host.c| 31 ++- >> >> arch/arm/plat-omap/include/plat/usb.h |7 + >> >> drivers/mfd/Kconfig |2 +- >> >> drivers/mfd/Makefile |2 +- >> >> drivers/mfd/omap-usb-host.c | 238 ++--- >> >> drivers/mfd/omap-usb-tll.c| 471 >> >> + >> >> 8 files changed, 523 insertions(+), 240 deletions(-) >> >> create mode 100644 drivers/mfd/omap-usb-tll.c >> >> >> >> -- >> >> 1.7.9.5 >> > >> > >> > Felipe/ Paul >> > please let me know if you have any review comments on this v4 series. >> > >> > regards >> > keshava >> >> Hi Felipe >> please let me know if you have any review comments on this series >> now. > > looks ok... pretty much just moving code around. > > -- > balbi Thanks Felipe then I request please give your Ack by for this series. regards keshava -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RESEND 1/4] ARM: OMAP2+: AM33XX: Add tps65910 device tree data
On 23/07/12 14:34, Mark Brown wrote: On Mon, Jul 23, 2012 at 01:23:50PM +, AnilKumar, Chimata wrote: By the way, if we look at all the regulator added (DT supported) till now have the similar problem. arch/arm/boot/dts/imx6q.dtsi This is fine - the SoC contains integrated regulators which supply other bits of the Soc so we can be confident that the hookup is good just based on the silicon. arch/arm/boot/dts/twl4030.dtsi arch/arm/boot/dts/twl6030.dtsi These appear to have similar issues and should be fixed, at least as far as the voltage ranges go. arch/arm/boot/dts/db8500.dtsi I'm not actually seeing anything terribly problematic here, though the regulator-name properties should really be removed as they're fairly useless and seem to be missing the point of having the property. I've missed the context of the thread, so can't comment, but I'm happy to remove the regulator-name properties from db8500.dts. Adding to my TODO. -- Lee Jones Linaro ST-Ericsson Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] PWM: Add support for configuring polarity of PWM
On Mon, Jul 23, 2012 at 18:38:25, Thierry Reding wrote: > On Mon, Jul 23, 2012 at 12:51:11PM +, Philip, Avinash wrote: > > On Mon, Jul 23, 2012 at 14:00:32, Thierry Reding wrote: > > > On Wed, Jul 18, 2012 at 06:24:13PM +0530, Philip, Avinash wrote: > [...] > > > > + > > > > + if (test_bit(PWMF_ENABLED, &pwm->flags)) { > > > > + dev_err(pwm->chip->dev, > > > > + "Polarity configuration Failed!, PWM device > > > > enabled\n"); > > > > + return -EBUSY; > > > > + } > > > > > > Maybe something like: "polarity cannot be configured while PWM device is > > > enabled"? > > > > Ok I will update. > > > > > Though I'm not sure the error message is all that useful. I'd > > > expect the user driver to handle -EBUSY specially. > > > > On EBUSY, client driver has to rework on it. Nothing to be done from > > framework > > Exactly, so I think that if an error is displayed because the PWM has > been enabled, then that client (== user) driver should output an error > message, not the framework. Also, it really shouldn't happen because it > clearly is a driver problem that needs to be fixed. Ok. I will remove. > > > > > /* > > > > * pwm_enable - start a PWM output toggling > > > > */ > > > > @@ -37,6 +47,7 @@ struct pwm_chip; > > > > enum { > > > > PWMF_REQUESTED = 1 << 0, > > > > PWMF_ENABLED = 1 << 1, > > > > + PWMF_POLARITY_INVERSE = 1 << 2, > > > > > > This should be named PWMF_POLARITY_INVERSED for consistency. > > > > Ok I will correct it. > > > > > I'm not sure that we really need this flag, though. It isn't used > > > anywhere. But > > > maybe you have a use-case in mind? > > > > It can be used to find the polarity of the PWM at runtime. > > Yes, but is there any use-case where this information would be required? It's been added as a feature enhancement. May be it can ignore? Thanks Avinash > > Thierry > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC RESEND 1/4] arm/dts: OMAP: Add timer nodes
Hi Rob, On 07/16/2012 10:56 AM, Jon Hunter wrote: > Hi Rob, > > On 07/13/2012 09:15 PM, Rob Herring wrote: >> On 07/13/2012 05:26 PM, Jon Hunter wrote: >>> Add the 12 GP timers nodes present in OMAP3. >>> Add the 11 GP timers nodes present in OMAP4. >>> >>> Add documentation for timer properties specific to OMAP. >>> >>> For each timer an alias is being added. The purpose for doing this is >>> because >>> the OMAP dmtimer driver uses an ID to distinguish between the different >>> timer >>> instances. For example, a timer can be requested by its ID. By adding an >>> alias >>> for each timer we can then use the function of_alias_get_id() to extract the >>> ID for each timer from the alias name. The same method is used for the TTY >>> serial devices. If it is preferred that such an alias is not added and there >>> is a better way to pass an ID from device-tree let me know. >> >> I'm not sure this is really a good use of aliases. UARTs use aliases >> because it is important that the UART number to tty number is known and >> fixed. IIUC, as an example you are picking timer1 because it has >> properties X, Y and Z. If so, then you should describe those h/w >> properties within the timer nodes so you can pick which timer to use >> based on it's h/w properties. > > Thanks for the feedback. What you suggest could definitely work for most > timers. The only item that I would need to resolve here is the handling > of system timers (ie. those used for clockevents and clocksource). These > system timers (for OMAP) are reserved during early boot based upon the > timer ID today and so this is before the actual main timer driver has > been probed and all the attributes of the timers has been read for > device-tree. > > One thought would be to move the reservation of the system timers out of > the kernel and into device-tree itself. Then we query device tree on > start-up to see which we should use. I am wondering if this could be a > better use of alias? For example, say I want to use timer1 as my > clockevent timer and so I could have an alias of ... > > alias { > clockevent_timer = &timer1; > } > > However, I am not sure if this is even correct, because there does not > appear to be an API to search the aliases by name and return the > phandle, just of_alias_get_id(). Alternatively, I could add another > property called "ti,timer-clockevent" that is populated for the timer > used as the clockevent timer. Do you have any inputs on the above? Does it make sense to reserve timer resources for kernel system timers in device-tree? Cheers Jon -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 19/25] of: add const to struct *of_device_id.data
On Mon, Jul 23, 2012 at 11:13:24AM +0200, Uwe Kleine-König wrote: > Drivers should never need to modify the data of a device id. So it can > be const which in turn allows more consts in the driver. > > Signed-off-by: Uwe Kleine-König Acked-by: Greg Kroah-Hartman -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 12/25] mmc/omap_hsmmc: add a const qualifier
On Mon, Jul 23, 2012 at 2:43 PM, Uwe Kleine-König wrote: > This prepares *of_device_id.data becoming const. Without this change > the following warning would occur: > > drivers/mmc/host/omap_hsmmc.c: In function 'omap_hsmmc_probe': > drivers/mmc/host/omap_hsmmc.c:1808: warning: initialization discards > qualifiers from pointer target type > > Signed-off-by: Uwe Kleine-König Acked-by: Venkatraman S > --- > drivers/mmc/host/omap_hsmmc.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: Current state of AM33xx patches
> -Original Message- > From: Daniel Mack [mailto:zon...@gmail.com] > Sent: Monday, July 23, 2012 12:01 PM > To: N, Mugunthan V > Cc: Hiremath, Vaibhav; Paul Walmsley; Nori, Sekhar; Jason Kridner; Tony > Lindgren; Hilman, Kevin; Hernandez, Carlos; Maupin, Chase; linux- > o...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; Kridner, Jason > Subject: Re: Current state of AM33xx patches > > On 05.07.2012 19:45, N, Mugunthan V wrote: > >> -Original Message- > >> From: Daniel Mack [mailto:zon...@gmail.com] > >> Sent: Wednesday, July 04, 2012 4:52 PM > >> To: Hiremath, Vaibhav > >> Cc: Paul Walmsley; Nori, Sekhar; Jason Kridner; Tony Lindgren; Hilman, > >> Kevin; Hernandez, Carlos; Maupin, Chase; linux-omap@vger.kernel.org; > >> linux-arm-ker...@lists.infradead.org; Kridner, Jason; N, Mugunthan V > >> Subject: Re: Current state of AM33xx patches > >> > >> On 04.07.2012 12:50, Hiremath, Vaibhav wrote: > >>> On Wed, Jul 04, 2012 at 15:37:31, Paul Walmsley wrote: > On Sat, 30 Jun 2012, Daniel Mack wrote: > > > Ok, thanks for the explaining this. For now, I'll add hwmod stubs > for > > the components I need. > > Hmm. Which drivers are you working on? Vaibhav Hiremath has hwmod > >> data > for the AM33xx. There might be some missing integration code to > build > >> the > devices for the newer IP blocks, though. > > >>> > >>> I would also be interested to know more here. > >>> > >>> > > Btw, has anyone yet worked on getting the MDIO/EMAC driver merged? > > Mainline has: > > drivers/net/ethernet/ti/davinci_emac.c > >>> > >>> Not required for AM335X. > >>> > drivers/net/ethernet/ti/davinci_mdio.c > > Those might work for you. > > >>> > >>> Few more files, > >>> > >>> drivers/net/ethernet/ti/cpsw.c > >>> drivers/net/ethernet/ti/davinci_cpdma.c > >>> > >>> > >>> Wanted to highlight one point, > >>> You still have to add platform device code to get it working. > >> > >> Exactly. And a hwmod binding for DT. Do you have patches for that? > >> > >> > >> Daniel > > > > I am working on DT support for cpsw and will be submitting the patch by > > July 20, 2012 > > Just curious - have you made any progress on that yet? > I have got the runtime PM support accepted to net-next, will be working on and submitting DT support by this week. Regards Mugunthan V N -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/25] i2c/i2c-omap: add a const qualifier
On Mon, Jul 23, 2012 at 2:43 PM, Uwe Kleine-König wrote: > This prepares *of_device_id.data becoming const. Without this change > the following warning would occur: > > drivers/i2c/busses/i2c-omap.c: In function 'omap_i2c_probe': > drivers/i2c/busses/i2c-omap.c:1025: warning: assignment discards > qualifiers from pointer target type > > Signed-off-by: Uwe Kleine-König Reviewed-by: Shubhrajyoti D > --- > drivers/i2c/busses/i2c-omap.c |3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv7 09/12] ARM: OMAP4: hwmod data: temporarily comment out data for the sl2if IP block
Hi Tero On Thu, 19 Jul 2012, Tero Kristo wrote: > The OMAP4 sl2if IP block requires some special programming for it to > enter idle. Without this programming, they will prevent the rest of > the chip from entering full chip idle. > > This patch comments out the IP block data. > > Later, once the appropriate support is available, this patch can be > reverted. > > Cc: Paul Walmsley > Cc: Benoît Cousson > Signed-off-by: Tero Kristo Originally I was hoping that we could apply a real fix for this one. But looking at the proposed alternative in the old SL2IF E-mail threads, I'm still not convinced by what's proposed there. So we'd better take this one for the time being. Queued for 3.6-rc. - Paul
Re: [PATCH] PWM: Add support for configuring polarity of PWM
On Mon, Jul 23, 2012 at 03:05:26PM +, Philip, Avinash wrote: > On Mon, Jul 23, 2012 at 18:38:25, Thierry Reding wrote: > > On Mon, Jul 23, 2012 at 12:51:11PM +, Philip, Avinash wrote: > > > On Mon, Jul 23, 2012 at 14:00:32, Thierry Reding wrote: > > > > On Wed, Jul 18, 2012 at 06:24:13PM +0530, Philip, Avinash wrote: > > > > > /* > > > > > * pwm_enable - start a PWM output toggling > > > > > */ > > > > > @@ -37,6 +47,7 @@ struct pwm_chip; > > > > > enum { > > > > > PWMF_REQUESTED = 1 << 0, > > > > > PWMF_ENABLED = 1 << 1, > > > > > + PWMF_POLARITY_INVERSE = 1 << 2, > > > > > > > > This should be named PWMF_POLARITY_INVERSED for consistency. > > > > > > Ok I will correct it. > > > > > > > I'm not sure that we really need this flag, though. It isn't used > > > > anywhere. But > > > > maybe you have a use-case in mind? > > > > > > It can be used to find the polarity of the PWM at runtime. > > > > Yes, but is there any use-case where this information would be required? > > It's been added as a feature enhancement. May be it can ignore? I think it can be removed for now. It can be added back if we ever really need it. Thierry pgpSXgB0z5MNn.pgp Description: PGP signature
Re: [PATCH] PWM: Add support for configuring polarity of PWM
On 07/23/2012 10:30 AM, Thierry Reding wrote: > On Wed, Jul 18, 2012 at 06:24:13PM +0530, Philip, Avinash wrote: >>[...] >> diff --git a/include/linux/pwm.h b/include/linux/pwm.h >> index 21d076c..2e4e960 100644 >> --- a/include/linux/pwm.h >> +++ b/include/linux/pwm.h >> @@ -21,6 +21,16 @@ void pwm_free(struct pwm_device *pwm); >> */ >> int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns); >> >> +enum { >> +PWM_POLARITY_NORMAL,/* ON period depends on duty_ns */ >> +PWM_POLARITY_INVERSE, /* OFF period depends on duty_ns */ >> +}; > > You should name this enumeration so that it can actually be used as a > type (enum pwm_polarity). Also you can drop the comments because they > only apply to the specific use-case of simulating duty-cycle inversion I think we should make it very explicit what normal polarity and inverse polarity is. There are certain applications where it is important. E.g. one such application would be using it in the IIO framework to generate a trigger pulse to synchronize devices. If we do not specify how each of these modes should behave drivers may interpret and implement them differently. I'd vote for the following definitions: PWM_POLARITY_NORMAL: A high signal for the duration of duty_ns, followed by a low signal for the duration of (period_ns - duty_ns). PWM_POLARITY_INVERSE: A low signal for the duration duty_ns, followed by a high signal for the duration of (period_ns - duty_ns). Maybe even rename them to PWM_POLARITY_ACTIVE_HIGH and PWM_POLARITY_ACTIVE_LOW since it is a bit more explicit on how the waveform should look like. "NORMAL" and "INVERSE" sort of depend on what you consider to be normal. - Lars -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V3 1/2] of: Add generic device tree DMA helpers
On 07/17/2012 01:24 PM, Arnd Bergmann wrote: ... > I think we're basically on the same page. Let's see if I have covered > all the cases we discussed so far. I've tried to update the binding that > Jon sent out initially with everything we've discussed, so please review > this to see if I understood you correctly. ... > * DMA client ... > Examples: ... > 3. A device with three channels, one of which has two alternatives: s/three/four/ s/one of which/both of which/ This binding doc seems reasonable to me. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: bisected regression: arm: omap3: voltage: fix channel configuration
On Mon, 23 Jul 2012 17:24:54 +0300 Tero Kristo wrote: > Hi Neil, > > On Mon, 2012-07-23 at 21:06 +1000, NeilBrown wrote: > > My GTA04 (mobile phone using OMAP3 and TWL4030 PMIC) has difficulty > > rebooting > > with 3.5. > > The first boot (after power on) works fine. But when I reboot it hangs just > > before > > > > Switched to new clocking rate (Crystal/Core/MPU): 26.0/332/800 MHz > > > > is printed. If I remove "mpurate=800" from the kernel command line the > > problem goes away. > > > > If I boot into 3.5, which works, and then reboot into an older working > > kernel > > it freezes too. So the 3.5 kernel leaves the device in a state which causes > > any kernel to hang. > > TWL chip retains its settings over warm boot which most likely explains > this. > > > > > I spent a while bisecting and narrowed it down to > > > > commit f9d29f1617eb1b2f1fd41622bd1a0fc51658d2f0 > > Author: Tero Kristo > > Date: Mon Feb 20 12:26:06 2012 +0200 > > > > arm: omap3: voltage: fix channel configuration > > > > > > If I revert this patch then the problem goes away. So that is the fix I'll > > go with for now, but if anyone wants me to try something else to help find > > out what the real problem is and to find a proper fix, I'm happy to help. > > Do you have cpufreq enabled in your build? If yes, then most likely the > crash occurs because cpufreq scales the used OPP to minimum after boot > and the warm reset doesn't switch the voltages properly and crashes as > it attempts to raise CPU frequency. Yes, I have cpufreq enabled. CONFIG_ARCH_HAS_CPUFREQ=y CONFIG_ARM_OMAP2PLUS_CPUFREQ=y Interestingly cpufreq thinks the range is 300MHz to 600MHz: /sys/devices/system/cpu/cpu0/cpufreq/scaling_available_frequencies:30 60 where 600 is that the freq is at boot: # dmesg | grep Cryst [0.00] Clocking rate (Crystal/Core/MPU): 26.0/332/600 MHz [0.056365] Switched to new clocking rate (Crystal/Core/MPU): 26.0/332/800 MHz Does that mean the mpurate=800 isn't doing anything at all? > > Also, if cpufreq is enabled, mpurate command line option doesn't really > do much... it will just switch the frequency to the desired target > during beginning of boot until cpufreq kicks in and switches the OPP to > its own understanding of desired cpu rate. I think the handling of > mpurate command line option should probably be ignored in cpufreq > builds, or moved within the cpufreq driver. So it looks like you are suggesting that I simply remove the "mpurate=800" as it isn't doing anything useful anyway. Is that right? Might there be some way to get it to scale higher than 600MHz? The first message from U-boot says: OMAP3630/3730-GP ES2.1, CPU-OPP2, L3-165MHz, Max CPU Clock 1 Ghz and the board manufacturer thinks it should be capable of 800MHz. Thanks, NeilBrown > > I tried to craft a quick boot time fix for this problem, but it seems > the voltage layer is not properly initialized yet when the mpurate > setting kicks in and it is not very straightforward to scale voltage > during this time without writing a serious hack. > > If you don't have cpufreq enabled, then the vdd routing on your board > might be interesting (I don't think this is the case as the kernel works > okay after 1st boot.) > > -Tero > signature.asc Description: PGP signature
NFS mount issue with OMAP4 on linux-next
Hi, I am not able to mount a filesyatem via NFS on OMAP4 SDP with the current linux-next kernel. The same thing works fine with OMAP3 SDP. Some more info: - initramfs works fine with OMAP4 SDP. - NFS mount on OMAP4 SDP works fine with mainline kernel. - Logs below (No output on console after the last print, but console takes input) Thanks, Archit [0.00] Booting Linux on physical CPU 0 [0.00] Linux version 3.5.0-next-20120723 (a0393947@a0393947pc) (gcc version 4.4.1 (Sourcery G++ Lite 2010q1-202) ) #68 SMP Tue Jul 24 11:30:19 IST 2012 [0.00] CPU: ARMv7 Processor [411fc092] revision 2 (ARMv7), cr=10c53c7d [0.00] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache [0.00] Machine: OMAP4430 4430SDP board [0.00] Memory policy: ECC disabled, Data cache writealloc [0.00] OMAP4430 ES2.1 [0.00] PERCPU: Embedded 9 pages/cpu @c1526000 s12736 r8192 d15936 u36864 [0.00] Built 1 zonelists in Zone order, mobility grouping on. Total pages: 259840 [0.00] Kernel command line: mem=1G console=ttyO2,115200n8 noinitrd root=/dev/nfs rw nfsroot=172.24.137.248:/nfs-share/mnt/,nolock,tcp,rsize=2048,wsize=2048,tcp ip=172.24.190.136 no_console_suspend [0.00] PID hash table entries: 4096 (order: 2, 16384 bytes) [0.00] Dentry cache hash table entries: 131072 (order: 7, 524288 bytes) [0.00] Inode-cache hash table entries: 65536 (order: 6, 262144 bytes) [0.00] Memory: 759MB 264MB = 1023MB total [0.00] Memory: 1025032k/1025032k available, 23544k reserved, 270336K highmem [0.00] Virtual kernel memory layout: [0.00] vector : 0x - 0x1000 ( 4 kB) [0.00] fixmap : 0xfff0 - 0xfffe ( 896 kB) [0.00] vmalloc : 0xf000 - 0xff00 ( 240 MB) [0.00] lowmem : 0xc000 - 0xef80 ( 760 MB) [0.00] pkmap : 0xbfe0 - 0xc000 ( 2 MB) [0.00] modules : 0xbf00 - 0xbfe0 ( 14 MB) [0.00] .text : 0xc0008000 - 0xc06ca0c4 (6921 kB) [0.00] .init : 0xc06cb000 - 0xc071a1c0 ( 317 kB) [0.00] .data : 0xc071c000 - 0xc07b8000 ( 624 kB) [0.00].bss : 0xc07b8024 - 0xc0d1260c (5482 kB) [0.00] Hierarchical RCU implementation. [0.00] NR_IRQS:474 [0.00] omap_hwmod: dpll_mpu_m2_ck: missing clockdomain for dpll_mpu_m2_ck. [0.00] OMAP clockevent source: GPTIMER1 at 32768 Hz [0.00] omap_hwmod: sys_32k_ck: missing clockdomain for sys_32k_ck. [0.00] sched_clock: 32 bits at 32kHz, resolution 30517ns, wraps every 131071999ms [0.00] OMAP clocksource: 32k_counter at 32768 Hz [0.00] Console: colour dummy device 80x30 [0.00] Lock dependency validator: Copyright (c) 2006 Red Hat, Inc., Ingo Molnar [0.00] ... MAX_LOCKDEP_SUBCLASSES: 8 [0.00] ... MAX_LOCK_DEPTH: 48 [0.00] ... MAX_LOCKDEP_KEYS:8191 [0.00] ... CLASSHASH_SIZE: 4096 [0.00] ... MAX_LOCKDEP_ENTRIES: 16384 [0.00] ... MAX_LOCKDEP_CHAINS: 32768 [0.00] ... CHAINHASH_SIZE: 16384 [0.00] memory used by lock dependency info: 3695 kB [0.00] per task-struct memory footprint: 1152 bytes [0.000823] Calibrating delay loop... 2007.19 BogoMIPS (lpj=7839744) [0.054565] pid_max: default: 32768 minimum: 301 [0.055084] Security Framework initialized [0.055267] Mount-cache hash table entries: 512 [0.059051] CPU: Testing write buffer coherency: ok [0.059936] CPU0: thread -1, cpu 0, socket 0, mpidr 8000 [0.060272] Setting up static identity map for 0x804f96a0 - 0x804f9710 [0.060363] L310 cache controller enabled [0.060363] l2x0: 16 ways, CACHE_ID 0x41c4, AUX_CTRL 0x7e47, Cache size: 1048576 B [0.063385] CPU1: Booted secondary processor [0.130554] CPU1: thread -1, cpu 1, socket 0, mpidr 8001 [0.130615] CPU1: Unknown IPI message 0x0 [0.131103] Brought up 2 CPUs [0.131103] SMP: Total of 2 processors activated (4022.78 BogoMIPS). [0.139221] omap_hwmod: l3_div_ck: missing clockdomain for l3_div_ck. [0.143218] omap_hwmod: sl2if: cannot be enabled for reset (3) [0.147216] omap_hwmod: mcpdm: cannot be enabled for reset (3) [0.147338] omap_hwmod: cm_core_aon: cannot be enabled for reset (3) [0.147369] omap_hwmod: cm_core: cannot be enabled for reset (3) [0.151733] dummy: [0.152709] NET: Registered protocol family 16 [0.154144] DMA: preallocated 256 KiB pool for atomic coherent allocations [0.154571] GPMC revision 6.0 [0.154693] gpmc: irq-52 could not claim: err -22 [0.159576] OMAP GPIO hardware version 0.1 [0.166137] omap_mux_init: Add partition: #1: core, flags: 2 [0.167541] omap_mux_init: Add partition: #2: wkup, flags: 2 [0.171203] _omap_mux_get_by_name: Could not find
RE: [PATCH] PWM: Add support for configuring polarity of PWM
On Tue, Jul 24, 2012 at 01:22:49, Thierry Reding wrote: > On Mon, Jul 23, 2012 at 03:05:26PM +, Philip, Avinash wrote: > > On Mon, Jul 23, 2012 at 18:38:25, Thierry Reding wrote: > > > On Mon, Jul 23, 2012 at 12:51:11PM +, Philip, Avinash wrote: > > > > On Mon, Jul 23, 2012 at 14:00:32, Thierry Reding wrote: > > > > > On Wed, Jul 18, 2012 at 06:24:13PM +0530, Philip, Avinash wrote: > > > > > > /* > > > > > > * pwm_enable - start a PWM output toggling > > > > > > */ > > > > > > @@ -37,6 +47,7 @@ struct pwm_chip; > > > > > > enum { > > > > > > PWMF_REQUESTED = 1 << 0, > > > > > > PWMF_ENABLED = 1 << 1, > > > > > > + PWMF_POLARITY_INVERSE = 1 << 2, > > > > > > > > > > This should be named PWMF_POLARITY_INVERSED for consistency. > > > > > > > > Ok I will correct it. > > > > > > > > > I'm not sure that we really need this flag, though. It isn't used > > > > > anywhere. But > > > > > maybe you have a use-case in mind? > > > > > > > > It can be used to find the polarity of the PWM at runtime. > > > > > > Yes, but is there any use-case where this information would be required? > > > > It's been added as a feature enhancement. May be it can ignore? > > I think it can be removed for now. It can be added back if we ever > really need it. Ok. I will remove it. Thanks Avinash > > Thierry > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/4] ARM: OMAP2+: AM33XX: Add tps65910 device tree data
Add device tree data for tps65910 regulator by adding all tps65910 regulator nodes. Regulator is initialized based on compatible name provided in tps65910 DT file. All tps65910 PMIC regulator device tree nodes are placed in a separate device tree include file (tps65910.dtsi). This patch was tested on AM335x-EVM. Signed-off-by: AnilKumar Ch --- arch/arm/boot/dts/tps65910.dtsi | 86 +++ 1 file changed, 86 insertions(+) create mode 100644 arch/arm/boot/dts/tps65910.dtsi diff --git a/arch/arm/boot/dts/tps65910.dtsi b/arch/arm/boot/dts/tps65910.dtsi new file mode 100644 index 000..92693a8 --- /dev/null +++ b/arch/arm/boot/dts/tps65910.dtsi @@ -0,0 +1,86 @@ +/* + * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/ + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +/* + * Integrated Power Management Chip + * http://www.ti.com/lit/ds/symlink/tps65910.pdf + */ + +&tps { + compatible = "ti,tps65910"; + + regulators { + #address-cells = <1>; + #size-cells = <0>; + + vrtc_reg: regulator@0 { + reg = <0>; + regulator-compatible = "vrtc"; + }; + + vio_reg: regulator@1 { + reg = <1>; + regulator-compatible = "vio"; + }; + + vdd1_reg: regulator@2 { + reg = <2>; + regulator-compatible = "vdd1"; + }; + + vdd2_reg: regulator@3 { + reg = <3>; + regulator-compatible = "vdd2"; + }; + + vdd3_reg: regulator@4 { + reg = <4>; + regulator-compatible = "vdd3"; + }; + + vdig1_reg: regulator@5 { + reg = <5>; + regulator-compatible = "vdig1"; + }; + + vdig2_reg: regulator@6 { + reg = <6>; + regulator-compatible = "vdig2"; + }; + + vpll_reg: regulator@7 { + reg = <7>; + regulator-compatible = "vpll"; + }; + + vdac_reg: regulator@8 { + reg = <8>; + regulator-compatible = "vdac"; + }; + + vaux1_reg: regulator@9 { + reg = <9>; + regulator-compatible = "vaux1"; + }; + + vaux2_reg: regulator@10 { + reg = <10>; + regulator-compatible = "vaux2"; + }; + + vaux33_reg: regulator@11 { + reg = <11>; + regulator-compatible = "vaux33"; + }; + + vmmc_reg: regulator@12 { + reg = <12>; + regulator-compatible = "vmmc"; + }; + }; +}; -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/4] Add AM33XX regulators device tree data
This patch series add AM33XX regulators (tps65910/tps65217) device tree data to am335x-evm and am335x-bone dts files. These patches are based on Tony L "devel-dt" tree and tested on AM335x EVM and Bone devices. Changes from v1: - Incorporated all Mark Brown's review comments on v1 by * moving all the regulator constraints to corresponding DTS file * Regulator constraints are added according to board. - regulator name is added. AnilKumar Ch (4): ARM: OMAP2+: AM33XX: Add tps65910 device tree data ARM: OMAP2+: AM33XX: Add tps65217 device tree data arm/dts: Add tps65910 regulator DT data to am335x-evm.dts arm/dts: Add tps65217 regulator DT data to am335x-bone.dts arch/arm/boot/dts/am335x-bone.dts | 28 arch/arm/boot/dts/am335x-evm.dts | 28 arch/arm/boot/dts/tps65217.dtsi | 56 arch/arm/boot/dts/tps65910.dtsi | 86 + 4 files changed, 198 insertions(+) create mode 100644 arch/arm/boot/dts/tps65217.dtsi create mode 100644 arch/arm/boot/dts/tps65910.dtsi -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 4/4] arm/dts: Add tps65217 regulator DT data to am335x-bone.dts
Add tps65217 regulator device tree data to AM335x-Bone by adding regulator consumers with tightened constraints and regulator-name. TPS65217 regulator handle can be obtained by using this regulator name. This patch also add I2C node with I2C frequency and tps65217 PMIC I2C slave address. Signed-off-by: AnilKumar Ch --- arch/arm/boot/dts/am335x-bone.dts | 28 1 file changed, 28 insertions(+) diff --git a/arch/arm/boot/dts/am335x-bone.dts b/arch/arm/boot/dts/am335x-bone.dts index a9af4db..8fdf43a 100644 --- a/arch/arm/boot/dts/am335x-bone.dts +++ b/arch/arm/boot/dts/am335x-bone.dts @@ -18,3 +18,31 @@ reg = <0x8000 0x1000>; /* 256 MB */ }; }; + +&i2c1 { + clock-frequency = <40>; + + tps: tps@24 { + reg = <0x24>; + }; +}; + +/include/ "tps65217.dtsi" + +&dcdc2_reg { + /* VDD_MPU voltage limits 0.95V - 1.26V with +/-4% tolerance */ + regulator-name = "vdd_mpu"; + regulator-min-microvolt = <925000>; + regulator-max-microvolt = <1325000>; + regulator-boot-on; + regulator-always-on; +}; + +&dcdc3_reg { + /* VDD_CORE voltage limits 0.95V - 1.1V with +/-4% tolerance */ + regulator-name = "vdd_core"; + regulator-min-microvolt = <925000>; + regulator-max-microvolt = <115>; + regulator-boot-on; + regulator-always-on; +}; -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/4] ARM: OMAP2+: AM33XX: Add tps65217 device tree data
Add device tree data for tps65217 regulator by adding all tps65217 regulator nodes. Regulator is initialized based on compatible name provided in tps65217 DT file. All tps65217 PMIC regulator device tree nodes are placed in a separate device tree include file (tps65217.dtsi). This patch was tested on AM335x-Bone. Signed-off-by: AnilKumar Ch --- arch/arm/boot/dts/tps65217.dtsi | 56 +++ 1 file changed, 56 insertions(+) create mode 100644 arch/arm/boot/dts/tps65217.dtsi diff --git a/arch/arm/boot/dts/tps65217.dtsi b/arch/arm/boot/dts/tps65217.dtsi new file mode 100644 index 000..a632724 --- /dev/null +++ b/arch/arm/boot/dts/tps65217.dtsi @@ -0,0 +1,56 @@ +/* + * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/ + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +/* + * Integrated Power Management Chip + * http://www.ti.com/lit/ds/symlink/tps65217.pdf + */ + +&tps { + compatible = "ti,tps65217"; + + regulators { + #address-cells = <1>; + #size-cells = <0>; + + dcdc1_reg: regulator@0 { + reg = <0>; + regulator-compatible = "dcdc1"; + }; + + dcdc2_reg: regulator@1 { + reg = <1>; + regulator-compatible = "dcdc2"; + }; + + dcdc3_reg: regulator@2 { + reg = <2>; + regulator-compatible = "dcdc3"; + }; + + ldo1_reg: regulator@3 { + reg = <3>; + regulator-compatible = "ldo1"; + }; + + ldo2_reg: regulator@4 { + reg = <4>; + regulator-compatible = "ldo2"; + }; + + ldo3_reg: regulator@5 { + reg = <5>; + regulator-compatible = "ldo3"; + }; + + ldo4_reg: regulator@6 { + reg = <6>; + regulator-compatible = "ldo4"; + }; + }; +}; -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/4] arm/dts: Add tps65910 regulator DT data to am335x-evm.dts
Add tps65910 regulator device tree data to AM335x-EVM by adding regulator consumers with tightened constraints and regulator-name. TPS65910 regulator handle can be obtained by using this regulator name. This patch also add I2C node with I2C frequency and tps65910 PMIC I2C slave address. Signed-off-by: AnilKumar Ch --- arch/arm/boot/dts/am335x-evm.dts | 28 1 file changed, 28 insertions(+) diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts index d6a97d9..e4e1ccb 100644 --- a/arch/arm/boot/dts/am335x-evm.dts +++ b/arch/arm/boot/dts/am335x-evm.dts @@ -18,3 +18,31 @@ reg = <0x8000 0x1000>; /* 256 MB */ }; }; + +&i2c1 { + clock-frequency = <40>; + + tps: tps@2D { + reg = <0x2D>; + }; +}; + +/include/ "tps65910.dtsi" + +&vdd1_reg { + /* VDD_MPU voltage limits 0.95V - 1.26V with +/-4% tolerance */ + regulator-name = "vdd_mpu"; + regulator-min-microvolt = <912500>; + regulator-max-microvolt = <1312500>; + regulator-boot-on; + regulator-always-on; +}; + +&vdd2_reg { + /* VDD_CORE voltage limits 0.95V - 1.1V with +/-4% tolerance */ + regulator-name = "vdd_core"; + regulator-min-microvolt = <912500>; + regulator-max-microvolt = <115>; + regulator-boot-on; + regulator-always-on; +}; -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: NFS mount issue with OMAP4 on linux-next
Hi, Comparing with working logs, I see this difference: [1.502166] spi spi1.0: no RX DMA engine channel for McSPI [1.507904] omap2_mcspi omap2_mcspi.1: can't setup spi1.0, status -11 [1.514862] omap2_mcspi omap2_mcspi.1: can't create new device for ks8851 I guess that's why it isn't working? Archit On Tuesday 24 July 2012 11:37 AM, Archit Taneja wrote: Hi, I am not able to mount a filesyatem via NFS on OMAP4 SDP with the current linux-next kernel. The same thing works fine with OMAP3 SDP. Some more info: - initramfs works fine with OMAP4 SDP. - NFS mount on OMAP4 SDP works fine with mainline kernel. - Logs below (No output on console after the last print, but console takes input) Thanks, Archit [0.00] Booting Linux on physical CPU 0 [0.00] Linux version 3.5.0-next-20120723 (a0393947@a0393947pc) (gcc version 4.4.1 (Sourcery G++ Lite 2010q1-202) ) #68 SMP Tue Jul 24 11:30:19 IST 2012 [0.00] CPU: ARMv7 Processor [411fc092] revision 2 (ARMv7), cr=10c53c7d [0.00] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache [0.00] Machine: OMAP4430 4430SDP board [0.00] Memory policy: ECC disabled, Data cache writealloc [0.00] OMAP4430 ES2.1 [0.00] PERCPU: Embedded 9 pages/cpu @c1526000 s12736 r8192 d15936 u36864 [0.00] Built 1 zonelists in Zone order, mobility grouping on. Total pages: 259840 [0.00] Kernel command line: mem=1G console=ttyO2,115200n8 noinitrd root=/dev/nfs rw nfsroot=172.24.137.248:/nfs-share/mnt/,nolock,tcp,rsize=2048,wsize=2048,tcp ip=172.24.190.136 no_console_suspend [0.00] PID hash table entries: 4096 (order: 2, 16384 bytes) [0.00] Dentry cache hash table entries: 131072 (order: 7, 524288 bytes) [0.00] Inode-cache hash table entries: 65536 (order: 6, 262144 bytes) [0.00] Memory: 759MB 264MB = 1023MB total [0.00] Memory: 1025032k/1025032k available, 23544k reserved, 270336K highmem [0.00] Virtual kernel memory layout: [0.00] vector : 0x - 0x1000 ( 4 kB) [0.00] fixmap : 0xfff0 - 0xfffe ( 896 kB) [0.00] vmalloc : 0xf000 - 0xff00 ( 240 MB) [0.00] lowmem : 0xc000 - 0xef80 ( 760 MB) [0.00] pkmap : 0xbfe0 - 0xc000 ( 2 MB) [0.00] modules : 0xbf00 - 0xbfe0 ( 14 MB) [0.00] .text : 0xc0008000 - 0xc06ca0c4 (6921 kB) [0.00] .init : 0xc06cb000 - 0xc071a1c0 ( 317 kB) [0.00] .data : 0xc071c000 - 0xc07b8000 ( 624 kB) [0.00].bss : 0xc07b8024 - 0xc0d1260c (5482 kB) [0.00] Hierarchical RCU implementation. [0.00] NR_IRQS:474 [0.00] omap_hwmod: dpll_mpu_m2_ck: missing clockdomain for dpll_mpu_m2_ck. [0.00] OMAP clockevent source: GPTIMER1 at 32768 Hz [0.00] omap_hwmod: sys_32k_ck: missing clockdomain for sys_32k_ck. [0.00] sched_clock: 32 bits at 32kHz, resolution 30517ns, wraps every 131071999ms [0.00] OMAP clocksource: 32k_counter at 32768 Hz [0.00] Console: colour dummy device 80x30 [0.00] Lock dependency validator: Copyright (c) 2006 Red Hat, Inc., Ingo Molnar [0.00] ... MAX_LOCKDEP_SUBCLASSES: 8 [0.00] ... MAX_LOCK_DEPTH: 48 [0.00] ... MAX_LOCKDEP_KEYS:8191 [0.00] ... CLASSHASH_SIZE: 4096 [0.00] ... MAX_LOCKDEP_ENTRIES: 16384 [0.00] ... MAX_LOCKDEP_CHAINS: 32768 [0.00] ... CHAINHASH_SIZE: 16384 [0.00] memory used by lock dependency info: 3695 kB [0.00] per task-struct memory footprint: 1152 bytes [0.000823] Calibrating delay loop... 2007.19 BogoMIPS (lpj=7839744) [0.054565] pid_max: default: 32768 minimum: 301 [0.055084] Security Framework initialized [0.055267] Mount-cache hash table entries: 512 [0.059051] CPU: Testing write buffer coherency: ok [0.059936] CPU0: thread -1, cpu 0, socket 0, mpidr 8000 [0.060272] Setting up static identity map for 0x804f96a0 - 0x804f9710 [0.060363] L310 cache controller enabled [0.060363] l2x0: 16 ways, CACHE_ID 0x41c4, AUX_CTRL 0x7e47, Cache size: 1048576 B [0.063385] CPU1: Booted secondary processor [0.130554] CPU1: thread -1, cpu 1, socket 0, mpidr 8001 [0.130615] CPU1: Unknown IPI message 0x0 [0.131103] Brought up 2 CPUs [0.131103] SMP: Total of 2 processors activated (4022.78 BogoMIPS). [0.139221] omap_hwmod: l3_div_ck: missing clockdomain for l3_div_ck. [0.143218] omap_hwmod: sl2if: cannot be enabled for reset (3) [0.147216] omap_hwmod: mcpdm: cannot be enabled for reset (3) [0.147338] omap_hwmod: cm_core_aon: cannot be enabled for reset (3) [0.147369] omap_hwmod: cm_core: cannot be enabled for reset (3) [0.151733] dummy: [0.152709] NET: Registered protocol family 16 [0.154144] DMA: preal
Re: bisected regression: arm: omap3: voltage: fix channel configuration
On Tue, 2012-07-24 at 11:30 +1000, NeilBrown wrote: > On Mon, 23 Jul 2012 17:24:54 +0300 Tero Kristo wrote: > > > Hi Neil, > > > > On Mon, 2012-07-23 at 21:06 +1000, NeilBrown wrote: > > > My GTA04 (mobile phone using OMAP3 and TWL4030 PMIC) has difficulty > > > rebooting > > > with 3.5. > > > The first boot (after power on) works fine. But when I reboot it hangs > > > just > > > before > > > > > > Switched to new clocking rate (Crystal/Core/MPU): 26.0/332/800 MHz > > > > > > is printed. If I remove "mpurate=800" from the kernel command line the > > > problem goes away. > > > > > > If I boot into 3.5, which works, and then reboot into an older working > > > kernel > > > it freezes too. So the 3.5 kernel leaves the device in a state which > > > causes > > > any kernel to hang. > > > > TWL chip retains its settings over warm boot which most likely explains > > this. > > > > > > > > I spent a while bisecting and narrowed it down to > > > > > > commit f9d29f1617eb1b2f1fd41622bd1a0fc51658d2f0 > > > Author: Tero Kristo > > > Date: Mon Feb 20 12:26:06 2012 +0200 > > > > > > arm: omap3: voltage: fix channel configuration > > > > > > > > > If I revert this patch then the problem goes away. So that is the fix > > > I'll > > > go with for now, but if anyone wants me to try something else to help find > > > out what the real problem is and to find a proper fix, I'm happy to help. > > > > Do you have cpufreq enabled in your build? If yes, then most likely the > > crash occurs because cpufreq scales the used OPP to minimum after boot > > and the warm reset doesn't switch the voltages properly and crashes as > > it attempts to raise CPU frequency. > > Yes, I have cpufreq enabled. > CONFIG_ARCH_HAS_CPUFREQ=y > CONFIG_ARM_OMAP2PLUS_CPUFREQ=y > > Interestingly cpufreq thinks the range is 300MHz to 600MHz: > > /sys/devices/system/cpu/cpu0/cpufreq/scaling_available_frequencies:30 > 60 Yeah, this is the default setup for omap36xx. You can see current cpu frequency by checking the contents of scaling_cur_freq, it is most likely 300MHz. > > where 600 is that the freq is at boot: > > # dmesg | grep Cryst > [0.00] Clocking rate (Crystal/Core/MPU): 26.0/332/600 MHz > [0.056365] Switched to new clocking rate (Crystal/Core/MPU): 26.0/332/800 > MHz > > Does that mean the mpurate=800 isn't doing anything at all? It switches it temporarily to 800MHz, maybe for a few seconds or so. > > > > > Also, if cpufreq is enabled, mpurate command line option doesn't really > > do much... it will just switch the frequency to the desired target > > during beginning of boot until cpufreq kicks in and switches the OPP to > > its own understanding of desired cpu rate. I think the handling of > > mpurate command line option should probably be ignored in cpufreq > > builds, or moved within the cpufreq driver. > > So it looks like you are suggesting that I simply remove the "mpurate=800" > as it isn't doing anything useful anyway. Is that right? > > Might there be some way to get it to scale higher than 600MHz? > The first message from U-boot says: > > OMAP3630/3730-GP ES2.1, CPU-OPP2, L3-165MHz, Max CPU Clock 1 Ghz > > and the board manufacturer thinks it should be capable of 800MHz. You need to enable 800MHz OPP similarly to what is done in beagle_opp_init() in board-omap3beagle.c. I am not sure what your board is detected as, depends on your boot loader (check /proc/cpuinfo.) -Tero > > Thanks, > NeilBrown > > > > > > I tried to craft a quick boot time fix for this problem, but it seems > > the voltage layer is not properly initialized yet when the mpurate > > setting kicks in and it is not very straightforward to scale voltage > > during this time without writing a serious hack. > > > > If you don't have cpufreq enabled, then the vdd routing on your board > > might be interesting (I don't think this is the case as the kernel works > > okay after 1st boot.) > > > > -Tero > > > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PWM: Add support for configuring polarity of PWM
On Mon, Jul 23, 2012 at 10:15:07PM +0200, Lars-Peter Clausen wrote: > On 07/23/2012 10:30 AM, Thierry Reding wrote: > > On Wed, Jul 18, 2012 at 06:24:13PM +0530, Philip, Avinash wrote: > >>[...] > >> diff --git a/include/linux/pwm.h b/include/linux/pwm.h > >> index 21d076c..2e4e960 100644 > >> --- a/include/linux/pwm.h > >> +++ b/include/linux/pwm.h > >> @@ -21,6 +21,16 @@ void pwm_free(struct pwm_device *pwm); > >> */ > >> int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns); > >> > >> +enum { > >> + PWM_POLARITY_NORMAL,/* ON period depends on duty_ns */ > >> + PWM_POLARITY_INVERSE, /* OFF period depends on duty_ns */ > >> +}; > > > > You should name this enumeration so that it can actually be used as a > > type (enum pwm_polarity). Also you can drop the comments because they > > only apply to the specific use-case of simulating duty-cycle inversion > > I think we should make it very explicit what normal polarity and inverse > polarity is. There are certain applications where it is important. E.g. one > such application would be using it in the IIO framework to generate a trigger > pulse to synchronize devices. If we do not specify how each of these modes > should behave drivers may interpret and implement them differently. I agree, the definition should be on a physical level. > I'd vote for the following definitions: > PWM_POLARITY_NORMAL: A high signal for the duration of duty_ns, followed by a > low signal for the duration of (period_ns - duty_ns). > PWM_POLARITY_INVERSE: A low signal for the duration duty_ns, followed by a > high > signal for the duration of (period_ns - duty_ns). That's my understanding of normal vs. inversed as well. I haven't yet seen a formal definition of the standard PWM waveform, but I believe this describes the most common implementation. > Maybe even rename them to PWM_POLARITY_ACTIVE_HIGH and PWM_POLARITY_ACTIVE_LOW > since it is a bit more explicit on how the waveform should look like. "NORMAL" > and "INVERSE" sort of depend on what you consider to be normal. But aren't active-high and -low equally arbitrary? They don't make it obvious as to where the active period is, either. I think it'd be enough if we use your definitions above as comments for the enumerations. After all the important thing here is to have an unambiguous definition. And I think for consistency we should call it PWM_POLARITY_INVERSED, that is if we keep those two definitions. How about the following? /** * enum pwm_polarity - polarity of a PWM signal * @PWM_POLARITY_NORMAL: a high signal for the duration of the duty- * cycle, followed by a low signal for the remainder of the pulse * period * @PWM_POLARITY_INVERSED: a low signal for the duration of the duty- * cycle, followed by a high signal for the remainder of the pulse * period */ enum pwm_polarity { PWM_POLARITY_NORMAL, PWM_POLARITY_INVERSED, }; Thierry pgpYeoenNeiXI.pgp Description: PGP signature
Re: NFS mount issue with OMAP4 on linux-next
On Tue, Jul 24, 2012 at 11:48 AM, Archit Taneja wrote: > Hi, > > Comparing with working logs, I see this difference: > > [1.502166] spi spi1.0: no RX DMA engine channel for McSPI > [1.507904] omap2_mcspi omap2_mcspi.1: can't setup spi1.0, status -11 > [1.514862] omap2_mcspi omap2_mcspi.1: can't create new device for ks8851 > > I guess that's why it isn't working? > Most likely yes. SPI is needed for ethernet on OMAP4. Please include this patch in your build and check.. http://git.kernel.org/?p=linux/kernel/git/tmlind/linux-omap.git;a=commit;h=55936cdfaaf11ac352b56bc58e42d6661e65ee13 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html