Re: [PATCH] dma: edma: add device_slave_caps() support
On 07/23/2013 06:43 PM, Joel Fernandes wrote: Implement device_slave_caps(). EDMA has a limited number of slots. Slave drivers such as omap_hsmmc will query the driver to make sure they don't pass in more than these many scatter segments. Signed-off-by: Joel Fernandes jo...@ti.com --- Vinod, or Dan- If this patch looks ok, can you please merge in for -rc cycle? This patch is required to fix MMC support on AM33xx. This patch is blocking 3 other patches which fix various MMC things. Thanks! Notes: (1) this approach is temporary and only for -rc cycle to fix MMC on AM335x. It will be replace by the RFC series in future kernels: http://www.spinics.net/lists/arm-kernel/msg260094.html (2) Patch depends Vinod's patch at: http://permalink.gmane.org/gmane.linux.kernel/1525112 drivers/dma/edma.c |9 + 1 file changed, 9 insertions(+) diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c index 7222cbe..81d5429 100644 --- a/drivers/dma/edma.c +++ b/drivers/dma/edma.c @@ -517,6 +517,14 @@ static void edma_issue_pending(struct dma_chan *chan) spin_unlock_irqrestore(echan-vchan.lock, flags); } +static inline int edma_slave_caps(struct dma_chan *chan, + struct dma_slave_caps *caps) +{ + caps-max_sg_nr = MAX_NR_SG; Hm, what about the other fields? - 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] dma: edma: add device_slave_caps() support
On 07/24/2013 03:03 AM, Lars-Peter Clausen wrote: On 07/23/2013 06:43 PM, Joel Fernandes wrote: Implement device_slave_caps(). EDMA has a limited number of slots. Slave drivers such as omap_hsmmc will query the driver to make sure they don't pass in more than these many scatter segments. Signed-off-by: Joel Fernandes jo...@ti.com --- Vinod, or Dan- If this patch looks ok, can you please merge in for -rc cycle? This patch is required to fix MMC support on AM33xx. This patch is blocking 3 other patches which fix various MMC things. Thanks! Notes: (1) this approach is temporary and only for -rc cycle to fix MMC on AM335x. It will be replace by the RFC series in future kernels: http://www.spinics.net/lists/arm-kernel/msg260094.html (2) Patch depends Vinod's patch at: http://permalink.gmane.org/gmane.linux.kernel/1525112 drivers/dma/edma.c |9 + 1 file changed, 9 insertions(+) diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c index 7222cbe..81d5429 100644 --- a/drivers/dma/edma.c +++ b/drivers/dma/edma.c @@ -517,6 +517,14 @@ static void edma_issue_pending(struct dma_chan *chan) spin_unlock_irqrestore(echan-vchan.lock, flags); } +static inline int edma_slave_caps(struct dma_chan *chan, +struct dma_slave_caps *caps) +{ +caps-max_sg_nr = MAX_NR_SG; Hm, what about the other fields? Other fields are unused, the max segment size is supposed to be calculated given the address width and burst size. Since these can't be provided to get_caps, I have left it out for now. See: https://lkml.org/lkml/2013/3/6/464 Even if it did, the segment size itself is unused in the MMC driver that this is supposed to fix, unlike the number of segments which I'm populating above. If you know of a better way to populate max segment size, let me know. Thanks, -Joel -- 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] dma: edma: add device_slave_caps() support
On 07/24/2013 10:11 AM, Joel Fernandes wrote: On 07/24/2013 03:03 AM, Lars-Peter Clausen wrote: On 07/23/2013 06:43 PM, Joel Fernandes wrote: Implement device_slave_caps(). EDMA has a limited number of slots. Slave drivers such as omap_hsmmc will query the driver to make sure they don't pass in more than these many scatter segments. Signed-off-by: Joel Fernandes jo...@ti.com --- Vinod, or Dan- If this patch looks ok, can you please merge in for -rc cycle? This patch is required to fix MMC support on AM33xx. This patch is blocking 3 other patches which fix various MMC things. Thanks! Notes: (1) this approach is temporary and only for -rc cycle to fix MMC on AM335x. It will be replace by the RFC series in future kernels: http://www.spinics.net/lists/arm-kernel/msg260094.html (2) Patch depends Vinod's patch at: http://permalink.gmane.org/gmane.linux.kernel/1525112 drivers/dma/edma.c |9 + 1 file changed, 9 insertions(+) diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c index 7222cbe..81d5429 100644 --- a/drivers/dma/edma.c +++ b/drivers/dma/edma.c @@ -517,6 +517,14 @@ static void edma_issue_pending(struct dma_chan *chan) spin_unlock_irqrestore(echan-vchan.lock, flags); } +static inline int edma_slave_caps(struct dma_chan *chan, + struct dma_slave_caps *caps) +{ + caps-max_sg_nr = MAX_NR_SG; Hm, what about the other fields? Other fields are unused, the max segment size is supposed to be calculated given the address width and burst size. Since these can't be provided to get_caps, I have left it out for now. See: https://lkml.org/lkml/2013/3/6/464 The PL330 driver is similar in this regard, the maximum segment size also depends on address width and burst width. What I did for the get_slave_caps implementation is to set it to the minimum maximum size. E.g. in you case that should be SZ_64K - 1 (burstsize and addrwidth both set to 1). Even if it did, the segment size itself is unused in the MMC driver that this is supposed to fix, unlike the number of segments which I'm populating above. E.g. for ALSA we'll need to know the max segment size, so I think it doesn't hurt add this in this patch as well. And you should also initialize all the other fields, even though if there are no users yet. It will be really painful to write generic drivers using the dmaengine API if none of the dmaengine drivers actually initializes the caps struct properly. - 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] dma: edma: add device_slave_caps() support
On Jul 24, 2013, at 3:23 AM, Lars-Peter Clausen l...@metafoo.de wrote: On 07/24/2013 10:11 AM, Joel Fernandes wrote: On 07/24/2013 03:03 AM, Lars-Peter Clausen wrote: On 07/23/2013 06:43 PM, Joel Fernandes wrote: Implement device_slave_caps(). EDMA has a limited number of slots. Slave drivers such as omap_hsmmc will query the driver to make sure they don't pass in more than these many scatter segments. Signed-off-by: Joel Fernandes jo...@ti.com --- Vinod, or Dan- If this patch looks ok, can you please merge in for -rc cycle? This patch is required to fix MMC support on AM33xx. This patch is blocking 3 other patches which fix various MMC things. Thanks! Notes: (1) this approach is temporary and only for -rc cycle to fix MMC on AM335x. It will be replace by the RFC series in future kernels: http://www.spinics.net/lists/arm-kernel/msg260094.html (2) Patch depends Vinod's patch at: http://permalink.gmane.org/gmane.linux.kernel/1525112 drivers/dma/edma.c |9 + 1 file changed, 9 insertions(+) diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c index 7222cbe..81d5429 100644 --- a/drivers/dma/edma.c +++ b/drivers/dma/edma.c @@ -517,6 +517,14 @@ static void edma_issue_pending(struct dma_chan *chan) spin_unlock_irqrestore(echan-vchan.lock, flags); } +static inline int edma_slave_caps(struct dma_chan *chan, +struct dma_slave_caps *caps) +{ +caps-max_sg_nr = MAX_NR_SG; Hm, what about the other fields? Other fields are unused, the max segment size is supposed to be calculated given the address width and burst size. Since these can't be provided to get_caps, I have left it out for now. See: https://lkml.org/lkml/2013/3/6/464 The PL330 driver is similar in this regard, the maximum segment size also depends on address width and burst width. What I did for the get_slave_caps implementation is to set it to the minimum maximum size. E.g. in you case that should be SZ_64K - 1 (burstsize and addrwidth both set to 1). So you're setting max to minimum maximum size? Isn't that like telling the driver that its segments can't be bigger than this... Unless I'm missing something.. Even if it did, the segment size itself is unused in the MMC driver that this is supposed to fix, unlike the number of segments which I'm populating above. E.g. for ALSA we'll need to know the max segment size, so I think it doesn't hurt add this in this patch as well. For alsa it would dma only the minimum max size even if the dma controller could do more? And you should also initialize all the other fields, even though if there are no users yet. It will be really painful to write generic drivers using the dmaengine API if none of the dmaengine drivers actually initializes the caps struct properly. Ok sure. Thanks, -Joel - 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] dma: edma: add device_slave_caps() support
On 07/24/2013 10:28 AM, Fernandes, Joel wrote: On Jul 24, 2013, at 3:23 AM, Lars-Peter Clausen l...@metafoo.de wrote: On 07/24/2013 10:11 AM, Joel Fernandes wrote: On 07/24/2013 03:03 AM, Lars-Peter Clausen wrote: On 07/23/2013 06:43 PM, Joel Fernandes wrote: Implement device_slave_caps(). EDMA has a limited number of slots. Slave drivers such as omap_hsmmc will query the driver to make sure they don't pass in more than these many scatter segments. Signed-off-by: Joel Fernandes jo...@ti.com --- Vinod, or Dan- If this patch looks ok, can you please merge in for -rc cycle? This patch is required to fix MMC support on AM33xx. This patch is blocking 3 other patches which fix various MMC things. Thanks! Notes: (1) this approach is temporary and only for -rc cycle to fix MMC on AM335x. It will be replace by the RFC series in future kernels: http://www.spinics.net/lists/arm-kernel/msg260094.html (2) Patch depends Vinod's patch at: http://permalink.gmane.org/gmane.linux.kernel/1525112 drivers/dma/edma.c |9 + 1 file changed, 9 insertions(+) diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c index 7222cbe..81d5429 100644 --- a/drivers/dma/edma.c +++ b/drivers/dma/edma.c @@ -517,6 +517,14 @@ static void edma_issue_pending(struct dma_chan *chan) spin_unlock_irqrestore(echan-vchan.lock, flags); } +static inline int edma_slave_caps(struct dma_chan *chan, +struct dma_slave_caps *caps) +{ +caps-max_sg_nr = MAX_NR_SG; Hm, what about the other fields? Other fields are unused, the max segment size is supposed to be calculated given the address width and burst size. Since these can't be provided to get_caps, I have left it out for now. See: https://lkml.org/lkml/2013/3/6/464 The PL330 driver is similar in this regard, the maximum segment size also depends on address width and burst width. What I did for the get_slave_caps implementation is to set it to the minimum maximum size. E.g. in you case that should be SZ_64K - 1 (burstsize and addrwidth both set to 1). So you're setting max to minimum maximum size? Isn't that like telling the driver that its segments can't be bigger than this... Unless I'm missing something.. Yes. This is a limitation of the current slave_caps API. The maximum needs to be the maximum for all possible configurations. A specific configuration may allow a larger maximum. So we maybe have to extend the API to be able to query the limits for a certain configuration. Not sure what the best way would be to do that, either adding a config parameter to get_slave_caps or to break it into two functions like you proposed one for the static capabilities and one for the sg limits. Even if it did, the segment size itself is unused in the MMC driver that this is supposed to fix, unlike the number of segments which I'm populating above. E.g. for ALSA we'll need to know the max segment size, so I think it doesn't hurt add this in this patch as well. For alsa it would dma only the minimum max size even if the dma controller could do more? Yes, but I think 64k is still plenty enough for the max period size. The current davinci PCM driver even claims to only support up to 8k. - 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 1/4] mfd: twl6030-irq: migrate to IRQ threaded handler
On Tue, 23 Jul 2013, Grygorii Strashko wrote: From: Naga Venkata Srikanth V vnv.srika...@samsung.com 1) Removed request_irq() and replaced it with request_threaded_irq(). 2) Removed generic_handle_irq() and replaced it with handle_nested_irq(). Handling of these interrupts is nested, as we are handling an interrupt (for e.g rtc, mmc1) when we are still servicing TWL irq. 3) Removed I2C read-retry logic for the case when twl_i2c_read() is failed inside IRQ handler - there is no sense to do that, so just report an error and return. Signed-off-by: Naga Venkata Srikanth V vnv.srika...@samsung.com Signed-off-by: Oleg_Kosheliev oleg.koshel...@ti.com Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com --- drivers/mfd/twl6030-irq.c | 146 +++-- 1 file changed, 49 insertions(+), 97 deletions(-) Besides the points I mention below I like the way this patch is going. diff --git a/drivers/mfd/twl6030-irq.c b/drivers/mfd/twl6030-irq.c index 277a8db..b6030d9 100644 --- a/drivers/mfd/twl6030-irq.c +++ b/drivers/mfd/twl6030-irq.c @@ -90,7 +90,6 @@ static unsigned twl6030_irq_base; static int twl_irq; static bool twl_irq_wake_enabled; -static struct completion irq_event; static atomic_t twl6030_wakeirqs = ATOMIC_INIT(0); static int twl6030_irq_pm_notifier(struct notifier_block *notifier, @@ -131,95 +130,57 @@ static struct notifier_block twl6030_irq_pm_notifier_block = { }; /* - * This thread processes interrupts reported by the Primary Interrupt Handler. - */ -static int twl6030_irq_thread(void *data) +* Threaded irq handler for the twl6030 interrupt. +* We query the interrupt controller in the twl6030 to determine +* which module is generating the interrupt request and call +* handle_nested_irq for that module. +*/ +static irqreturn_t twl6030_irq_thread(int irq, void *data) { - long irq = (long)data; - static unsigned i2c_errors; - static const unsigned max_i2c_errors = 100; - int ret; - - while (!kthread_should_stop()) { - int i; - union { + int i, ret; + union { u8 bytes[4]; u32 int_sts; - } sts; - - /* Wait for IRQ, then read PIH irq status (also blocking) */ - wait_for_completion_interruptible(irq_event); - - /* read INT_STS_A, B and C in one shot using a burst read */ - ret = twl_i2c_read(TWL_MODULE_PIH, sts.bytes, - REG_INT_STS_A, 3); - if (ret) { - pr_warning(twl6030: I2C error %d reading PIH ISR\n, - ret); - if (++i2c_errors = max_i2c_errors) { - printk(KERN_ERR Maximum I2C error count - exceeded. Terminating %s.\n, - __func__); - break; - } - complete(irq_event); - continue; - } - + } sts; + /* read INT_STS_A, B and C in one shot using a burst read */ + ret = twl_i2c_read(TWL_MODULE_PIH, sts.bytes, REG_INT_STS_A, 3); + if (ret) { + pr_warn(%s: I2C error %d reading PIH ISR\n, __func__, ret); Does the user really care which function we're returning from. Would it be better if you replace '__func__' with the device name? + return IRQ_HANDLED; + } - sts.bytes[3] = 0; /* Only 24 bits are valid*/ + sts.bytes[3] = 0; /* Only 24 bits are valid*/ - /* - * Since VBUS status bit is not reliable for VBUS disconnect - * use CHARGER VBUS detection status bit instead. - */ - if (sts.bytes[2] 0x10) - sts.bytes[2] |= 0x08; + /* + * Since VBUS status bit is not reliable for VBUS disconnect + * use CHARGER VBUS detection status bit instead. + */ + if (sts.bytes[2] 0x10) + sts.bytes[2] |= 0x08; - for (i = 0; sts.int_sts; sts.int_sts = 1, i++) { - local_irq_disable(); - if (sts.int_sts 0x1) { - int module_irq = twl6030_irq_base + + for (i = 0; sts.int_sts; sts.int_sts = 1, i++) + if (sts.int_sts 0x1) { I'm a little confused by this. Where does sts.int_sts come from? + int module_irq = twl6030_irq_base + twl6030_interrupt_mapping[i]; - generic_handle_irq(module_irq); - - } - local_irq_enable(); + handle_nested_irq(module_irq); + pr_debug(%s: PIH ISR %u, virq%u\n, + __func__, i, module_irq); }
Re: [PATCH 3/4] mfd: twl6030-irq: convert to use linear irq_domain
On Tue, 23 Jul 2013, Grygorii Strashko wrote: Since the TWL6030 PMIC is used with OMAP4 SoCs only and OMAP4 legacy boot is dropped there are no needs to allocate the range of IRQ descriptors during system boot to support TWL6030 IRQs. Hence, convert it to use linear irq_domain and move IRQ configuration in .map()/.unmap() callbacks of irq_domain. So, IRQ mapping and descriptors allocation will be performed dynamically basing on DT configuration. The error message will be reported in case if unmapped IRQ is received by TWL6030 (virq==0). Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com --- drivers/mfd/twl6030-irq.c | 114 - 1 file changed, 61 insertions(+), 53 deletions(-) diff --git a/drivers/mfd/twl6030-irq.c b/drivers/mfd/twl6030-irq.c index 790cc28..89f130b 100644 --- a/drivers/mfd/twl6030-irq.c +++ b/drivers/mfd/twl6030-irq.c @@ -138,6 +138,7 @@ static struct notifier_block twl6030_irq_pm_notifier_block = { static irqreturn_t twl6030_irq_thread(int irq, void *data) { int i, ret; + struct irq_domain *irq_domain = (struct irq_domain *)data; union { u8 bytes[4]; u32 int_sts; @@ -161,9 +162,14 @@ static irqreturn_t twl6030_irq_thread(int irq, void *data) for (i = 0; sts.int_sts; sts.int_sts = 1, i++) if (sts.int_sts 0x1) { - int module_irq = twl6030_irq_base + - twl6030_interrupt_mapping[i]; - handle_nested_irq(module_irq); + int module_irq = + irq_find_mapping(irq_domain, + twl6030_interrupt_mapping[i]); + if (module_irq) + handle_nested_irq(module_irq); + else + pr_err(%s: Unmapped PIH ISR %u detected\n, +__func__, i); Same here. Does the user really care which function failed? Please consider replacing with the device name. pr_debug(%s: PIH ISR %u, virq%u\n, __func__, i, module_irq); } @@ -186,19 +192,6 @@ static irqreturn_t twl6030_irq_thread(int irq, void *data) /*--*/ -static inline void activate_irq(int irq) -{ -#ifdef CONFIG_ARM - /* ARM requires an extra step to clear IRQ_NOREQUEST, which it - * sets on behalf of every irq_chip. Also sets IRQ_NOPROBE. - */ - set_irq_flags(irq, IRQF_VALID); -#else - /* same effect on other architectures */ - irq_set_noprobe(irq); -#endif -} - static int twl6030_irq_set_wake(struct irq_data *d, unsigned int on) { if (on) @@ -308,28 +301,54 @@ int twl6030_mmc_card_detect(struct device *dev, int slot) } EXPORT_SYMBOL(twl6030_mmc_card_detect); +static struct irq_chip twl6030_irq_chip; + +static int twl6030_irq_map(struct irq_domain *d, unsigned int virq, + irq_hw_number_t hwirq) +{ + irq_set_chip_data(virq, twl6030_irq_chip); + irq_set_chip_and_handler(virq, twl6030_irq_chip, handle_simple_irq); + irq_set_nested_thread(virq, true); + +#ifdef CONFIG_ARM + /* + * ARM requires an extra step to clear IRQ_NOREQUEST, which it + * sets on behalf of every irq_chip. Also sets IRQ_NOPROBE. + */ + set_irq_flags(virq, IRQF_VALID); +#else + /* same effect on other architectures */ + irq_set_noprobe(virq); +#endif + + return 0; +} + +static void twl6030_irq_unmap(struct irq_domain *d, unsigned int virq) +{ +#ifdef CONFIG_ARM + set_irq_flags(virq, 0); +#endif + irq_set_chip_and_handler(virq, NULL, NULL); + irq_set_chip_data(virq, NULL); +} + +static struct irq_domain_ops twl6030_irq_domain_ops = { + .map= twl6030_irq_map, + .unmap = twl6030_irq_unmap, + .xlate = irq_domain_xlate_onetwocell, +}; + int twl6030_init_irq(struct device *dev, int irq_num) { struct device_node *node = dev-of_node; - int nr_irqs, irq_base, irq_end; - static struct irq_chip twl6030_irq_chip; + int nr_irqs; int status; - int i; u8 mask[3]; + struct irq_domain *irq_domain; nr_irqs = TWL6030_NR_IRQS; - irq_base = irq_alloc_descs(-1, 0, nr_irqs, 0); - if (IS_ERR_VALUE(irq_base)) { - dev_err(dev, Fail to allocate IRQ descs\n); - return irq_base; - } - - irq_domain_add_legacy(node, nr_irqs, irq_base, 0, - irq_domain_simple_ops, NULL); - - irq_end = irq_base + nr_irqs; - mask[0] = 0xFF; mask[1] = 0xFF; mask[2] =
Re: [PATCH 4/4] mfd: twl6030-irq: Add interrupt mapping table for the twl6032
On Tue, 23 Jul 2013, Grygorii Strashko wrote: From: Oleksandr Dmytryshyn oleksandr.dmytrys...@ti.com This patch adds interrupt mapping table for the twl6032. Repeating the $SUBJECT line is never helpful. Signed-off-by: Oleksandr Dmytryshyn oleksandr.dmytrys...@ti.com Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com --- drivers/mfd/twl6030-irq.c | 49 - 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/drivers/mfd/twl6030-irq.c b/drivers/mfd/twl6030-irq.c index 89f130b..e4df87f 100644 --- a/drivers/mfd/twl6030-irq.c +++ b/drivers/mfd/twl6030-irq.c @@ -41,6 +41,7 @@ #include linux/suspend.h #include linux/of.h #include linux/irqdomain.h +#include linux/of_device.h #include twl-core.h @@ -84,6 +85,36 @@ static int twl6030_interrupt_mapping[24] = { CHARGERFAULT_INTR_OFFSET, /* Bit 22 INT_CHRG*/ RSV_INTR_OFFSET,/* Bit 23 Reserved*/ }; + +static int twl6032_interrupt_mapping[24] = { + PWR_INTR_OFFSET,/* Bit 0PWRON */ + PWR_INTR_OFFSET,/* Bit 1RPWRON */ + PWR_INTR_OFFSET,/* Bit 2SYS_VLOW*/ + RTC_INTR_OFFSET,/* Bit 3RTC_ALARM */ + RTC_INTR_OFFSET,/* Bit 4RTC_PERIOD */ + HOTDIE_INTR_OFFSET, /* Bit 5HOT_DIE */ + SMPSLDO_INTR_OFFSET,/* Bit 6VXXX_SHORT */ + PWR_INTR_OFFSET,/* Bit 7SPDURATION */ + + PWR_INTR_OFFSET,/* Bit 8WATCHDOG*/ + BATDETECT_INTR_OFFSET, /* Bit 9BAT */ + SIMDETECT_INTR_OFFSET, /* Bit 10 SIM */ + MMCDETECT_INTR_OFFSET, /* Bit 11 MMC */ + MADC_INTR_OFFSET, /* Bit 12 GPADC_RT_EOC*/ + MADC_INTR_OFFSET, /* Bit 13 GPADC_SW_EOC*/ + GASGAUGE_INTR_OFFSET, /* Bit 14 CC_EOC */ + GASGAUGE_INTR_OFFSET, /* Bit 15 CC_AUTOCAL */ + + USBOTG_INTR_OFFSET, /* Bit 16 ID_WKUP */ + USBOTG_INTR_OFFSET, /* Bit 17 VBUS_WKUP */ + USBOTG_INTR_OFFSET, /* Bit 18 ID */ + USB_PRES_INTR_OFFSET, /* Bit 19 VBUS*/ + CHARGER_INTR_OFFSET,/* Bit 20 CHRG_CTRL */ + CHARGERFAULT_INTR_OFFSET, /* Bit 21 EXT_CHRG*/ + CHARGERFAULT_INTR_OFFSET, /* Bit 22 INT_CHRG*/ OCD failure. ;) NB: Kidding, you don't have to do anything about this. + RSV_INTR_OFFSET,/* Bit 23 Reserved*/ +}; + /*--*/ static unsigned twl6030_irq_base; @@ -91,6 +122,7 @@ static int twl_irq; static bool twl_irq_wake_enabled; static atomic_t twl6030_wakeirqs = ATOMIC_INIT(0); +static const int *irq_mapping_tbl; What I'd actually like to see is the creation of 'struct twl6030' to keep all your goodies in; irq_domain, irq_mapping_tbl etc and for you to pass that around instead of creating more global variables e.g. via request_threaded_irq(..., void *dev_id) to access the aforementioned information. static int twl6030_irq_pm_notifier(struct notifier_block *notifier, unsigned long pm_event, void *unused) @@ -164,7 +196,7 @@ static irqreturn_t twl6030_irq_thread(int irq, void *data) if (sts.int_sts 0x1) { int module_irq = irq_find_mapping(irq_domain, - twl6030_interrupt_mapping[i]); + irq_mapping_tbl[i]); if (module_irq) handle_nested_irq(module_irq); else @@ -339,6 +371,12 @@ static struct irq_domain_ops twl6030_irq_domain_ops = { .xlate = irq_domain_xlate_onetwocell, }; +static const struct of_device_id twl6030_of_match[] = { + {.compatible = ti,twl6030, twl6030_interrupt_mapping}, + {.compatible = ti,twl6032, twl6032_interrupt_mapping}, + { }, +}; + int twl6030_init_irq(struct device *dev, int irq_num) { struct device_node *node = dev-of_node; @@ -346,6 +384,15 @@ int twl6030_init_irq(struct device *dev, int irq_num) int status; u8 mask[3]; struct irq_domain *irq_domain; + const struct of_device_id *of_id; + + of_id = of_match_device(twl6030_of_match, dev); + if (!of_id || !of_id-data) { + dev_err(dev, Unknown TWL device model\n); +
Re: [PATCH 2/4] mfd: twl6030-irq: add error check when IRQs are masked initially
On 07/23/2013 09:08 PM, Graeme Gregory wrote: On 23/07/13 17:07, Grygorii Strashko wrote: Add a missed check for errors when TWL IRQs are masked initially on probe and report an error in case of failure. Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com --- drivers/mfd/twl6030-irq.c | 13 + 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/drivers/mfd/twl6030-irq.c b/drivers/mfd/twl6030-irq.c index b6030d9..790cc28 100644 --- a/drivers/mfd/twl6030-irq.c +++ b/drivers/mfd/twl6030-irq.c @@ -313,7 +313,7 @@ int twl6030_init_irq(struct device *dev, int irq_num) struct device_node *node = dev-of_node; int nr_irqs, irq_base, irq_end; static struct irq_chip twl6030_irq_chip; - int status = 0; + int status; int i; u8 mask[3]; @@ -335,11 +335,16 @@ int twl6030_init_irq(struct device *dev, int irq_num) mask[2] = 0xFF; /* mask all int lines */ - twl_i2c_write(TWL_MODULE_PIH, mask[0], REG_INT_MSK_LINE_A, 3); + status = twl_i2c_write(TWL_MODULE_PIH, mask[0], REG_INT_MSK_LINE_A, 3); /* mask all int sts */ - twl_i2c_write(TWL_MODULE_PIH, mask[0], REG_INT_MSK_STS_A, 3); + status |= twl_i2c_write(TWL_MODULE_PIH, mask[0], REG_INT_MSK_STS_A, 3); /* clear INT_STS_A,B,C */ - twl_i2c_write(TWL_MODULE_PIH, mask[0], REG_INT_STS_A, 3); + status |= twl_i2c_write(TWL_MODULE_PIH, mask[0], REG_INT_STS_A, 3); + You can save two i2c writes here for slightly faster initialisation, only one of the REG_INT_STS_A registers needs to be written to clear them all. As per the irq handling routine comment. Good point. thanks + if (status 0) { + dev_err(dev, I2C err writing TWL_MODULE_PIH: %d\n, status); + return status; + } twl6030_irq_base = irq_base; Graeme Regards, - grygorii -- 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/4] mfd: twl6030-irq: migrate to IRQ threaded handler
On Tue, 23 Jul 2013, Grygorii Strashko wrote: From: Naga Venkata Srikanth V vnv.srika...@samsung.com 1) Removed request_irq() and replaced it with request_threaded_irq(). 2) Removed generic_handle_irq() and replaced it with handle_nested_irq(). Handling of these interrupts is nested, as we are handling an interrupt (for e.g rtc, mmc1) when we are still servicing TWL irq. 3) Removed I2C read-retry logic for the case when twl_i2c_read() is failed inside IRQ handler - there is no sense to do that, so just report an error and return. Signed-off-by: Naga Venkata Srikanth V vnv.srika...@samsung.com Signed-off-by: Oleg_Kosheliev oleg.koshel...@ti.com Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com --- drivers/mfd/twl6030-irq.c | 146 +++-- 1 file changed, 49 insertions(+), 97 deletions(-) diff --git a/drivers/mfd/twl6030-irq.c b/drivers/mfd/twl6030-irq.c snip + status = request_threaded_irq(irq_num, NULL, twl6030_irq_thread, + IRQF_ONESHOT, TWL6030-PIH, NULL); Oh, and please use managed resources for this: devm_* if (status 0) { dev_err(dev, could not claim irq %d: %d\n, irq_num, status); goto fail_irq; } -- 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 1/4] mfd: twl6030-irq: migrate to IRQ threaded handler
On 07/24/2013 01:49 PM, Lee Jones wrote: On Tue, 23 Jul 2013, Grygorii Strashko wrote: From: Naga Venkata Srikanth V vnv.srika...@samsung.com 1) Removed request_irq() and replaced it with request_threaded_irq(). 2) Removed generic_handle_irq() and replaced it with handle_nested_irq(). Handling of these interrupts is nested, as we are handling an interrupt (for e.g rtc, mmc1) when we are still servicing TWL irq. 3) Removed I2C read-retry logic for the case when twl_i2c_read() is failed inside IRQ handler - there is no sense to do that, so just report an error and return. Signed-off-by: Naga Venkata Srikanth V vnv.srika...@samsung.com Signed-off-by: Oleg_Kosheliev oleg.koshel...@ti.com Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com --- drivers/mfd/twl6030-irq.c | 146 +++-- 1 file changed, 49 insertions(+), 97 deletions(-) Besides the points I mention below I like the way this patch is going. diff --git a/drivers/mfd/twl6030-irq.c b/drivers/mfd/twl6030-irq.c index 277a8db..b6030d9 100644 --- a/drivers/mfd/twl6030-irq.c +++ b/drivers/mfd/twl6030-irq.c @@ -90,7 +90,6 @@ static unsigned twl6030_irq_base; static int twl_irq; static bool twl_irq_wake_enabled; -static struct completion irq_event; static atomic_t twl6030_wakeirqs = ATOMIC_INIT(0); static int twl6030_irq_pm_notifier(struct notifier_block *notifier, @@ -131,95 +130,57 @@ static struct notifier_block twl6030_irq_pm_notifier_block = { }; /* - * This thread processes interrupts reported by the Primary Interrupt Handler. - */ -static int twl6030_irq_thread(void *data) +* Threaded irq handler for the twl6030 interrupt. +* We query the interrupt controller in the twl6030 to determine +* which module is generating the interrupt request and call +* handle_nested_irq for that module. +*/ +static irqreturn_t twl6030_irq_thread(int irq, void *data) { - long irq = (long)data; - static unsigned i2c_errors; - static const unsigned max_i2c_errors = 100; - int ret; - - while (!kthread_should_stop()) { - int i; - union { + int i, ret; + union { u8 bytes[4]; u32 int_sts; - } sts; - - /* Wait for IRQ, then read PIH irq status (also blocking) */ - wait_for_completion_interruptible(irq_event); - - /* read INT_STS_A, B and C in one shot using a burst read */ - ret = twl_i2c_read(TWL_MODULE_PIH, sts.bytes, - REG_INT_STS_A, 3); - if (ret) { - pr_warning(twl6030: I2C error %d reading PIH ISR\n, - ret); - if (++i2c_errors = max_i2c_errors) { - printk(KERN_ERR Maximum I2C error count -exceeded. Terminating %s.\n, - __func__); - break; - } - complete(irq_event); - continue; - } - + } sts; + /* read INT_STS_A, B and C in one shot using a burst read */ + ret = twl_i2c_read(TWL_MODULE_PIH, sts.bytes, REG_INT_STS_A, 3); sts.int_sts - is filled here + if (ret) { + pr_warn(%s: I2C error %d reading PIH ISR\n, __func__, ret); Does the user really care which function we're returning from. Would it be better if you replace '__func__' with the device name? This module hasn't been converted to the device yet:( (I mean interrupt-controller). But I'm thinking about it as the next step :) and then It will be absolutely reasonable change to replace pr_*() with dev_*() and remove __func__. Now, the pointer on dev (in our case twl-core device) isn't passed in IRQ handler, so It can't be used here. Of course it can be done, but would it make code better? My opinion - no. + return IRQ_HANDLED; + } - sts.bytes[3] = 0; /* Only 24 bits are valid*/ + sts.bytes[3] = 0; /* Only 24 bits are valid*/ - /* -* Since VBUS status bit is not reliable for VBUS disconnect -* use CHARGER VBUS detection status bit instead. -*/ - if (sts.bytes[2] 0x10) - sts.bytes[2] |= 0x08; + /* +* Since VBUS status bit is not reliable for VBUS disconnect +* use CHARGER VBUS detection status bit instead. +*/ + if (sts.bytes[2] 0x10) + sts.bytes[2] |= 0x08; - for (i = 0; sts.int_sts; sts.int_sts = 1, i++) { - local_irq_disable(); - if (sts.int_sts 0x1) { - int module_irq = twl6030_irq_base + + for (i = 0; sts.int_sts; sts.int_sts = 1, i++) + if (sts.int_sts 0x1) {
Re: [PATCH 1/4] mfd: twl6030-irq: migrate to IRQ threaded handler
+ if (ret) { + pr_warn(%s: I2C error %d reading PIH ISR\n, __func__, ret); Does the user really care which function we're returning from. Would it be better if you replace '__func__' with the device name? This module hasn't been converted to the device yet:( (I mean interrupt-controller). But I'm thinking about it as the next step :) and then It will be absolutely reasonable change to replace pr_*() with dev_*() and remove __func__. I don't mean anything as compicated as that for 'this' patch. (NB: See my comment in subsequent patches about creating a 'struct twl6030' where you could store 'struct dev'.) In this patch I mean litterally replacing %s: , with tw16030_irq: . Simples. :) Now, the pointer on dev (in our case twl-core device) isn't passed in IRQ handler, so It can't be used here. Of course it can be done, but would it make code better? My opinion - no. + if (sts.bytes[2] 0x10) + sts.bytes[2] |= 0x08; - for (i = 0; sts.int_sts; sts.int_sts = 1, i++) { - local_irq_disable(); - if (sts.int_sts 0x1) { - int module_irq = twl6030_irq_base + + for (i = 0; sts.int_sts; sts.int_sts = 1, i++) + if (sts.int_sts 0x1) { I'm a little confused by this. Where does sts.int_sts come from? See my comment above, pls Okay, that's my fault for not understanding unions properly as I've never had to use one, but now I do, thanks. @@ -437,10 +386,13 @@ int twl6030_exit_irq(void) { unregister_pm_notifier(twl6030_irq_pm_notifier_block); - if (twl6030_irq_base) { + if (!twl6030_irq_base) { pr_err(twl6030: can't yet clean up IRQs?\n); return -ENOSYS; } + + free_irq(twl_irq, NULL); + If request_threaded_irq() fails, isn't there a chance that twl6030_irq_base will be allocated, but twl_irq will still be undefined? Yes. A mess is here (historically:), thanks. Will use twl_irq instead of twl6030_irq_base (I did it, actually, in patch [3]:). Yes, I saw it. It would be better if you still fixed up this patch to be correct though. Even if you break it out and add it as [PATCH 1/x]. -- 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] arm: dts: twl6030: typical connection to omap4 as a separate dtsi file
Hi Uri, On 07/24/2013 10:05 AM, Uri Yosef wrote: Hi Ruslan, Our OMAP4 SOM connection to PMIC is similar to Blaze SDP4430. Thank you for confirmation, I will add this in next patch series. Regards, Ruslan Regards, Uri Yosef On Mon, Jul 22, 2013 at 7:24 PM, Ruslan Bilovolruslan.bilo...@ti.comwrote: Hi Nishanth, On Fri, Jul 19, 2013 at 5:10 PM, Nishanth Menonn...@ti.com wrote: On 07/19/2013 07:10 AM, Ruslan Bilovol wrote: The OMAP4 SoC family uses specially-designed PMIC (power management IC) companion chip for power management needs: TWL6030/TWL6032. Therefore there is a typical connection of PMIC to OMAP4 so we can figure it out into separate .dtsi file and do not duplicate over board-specific files. Tested on OMAP4 SDP board and compile-tested for Panda board Signed-off-by: Ruslan Bilovolruslan.bilo...@ti.com --- arch/arm/boot/dts/omap4-panda-common.dtsi | 21 +--- arch/arm/boot/dts/omap4-sdp.dts | 21 +--- arch/arm/boot/dts/twl6030_omap4.dtsi | 38 + arch/arm/boot/dts/omap4-var-som.dts ? There is no muxing for TWL6030 in that file at all, so I didn't add it. Moreover, that file seems to be not maintained since added (just generic fixes or framework changes that came to all other files) Second thing - I do not have schematics and HW for verification. But if somebody from Variscite can confirm that omap4 is typically connected to PMIC, I'm happy to add it as separate patch. So I CCed Uri Yosef who is original author of the omap4-var-som.dts Regards, Ruslan 3 files changed, 40 insertions(+), 40 deletions(-) create mode 100644 arch/arm/boot/dts/twl6030_omap4.dtsi diff --git a/arch/arm/boot/dts/omap4-panda-common.dtsi b/arch/arm/boot/dts/omap4-panda-common.dtsi index faa95b5..d796302 100644 --- a/arch/arm/boot/dts/omap4-panda-common.dtsi +++ b/arch/arm/boot/dts/omap4-panda-common.dtsi @@ -109,23 +109,9 @@ }; }; -omap4_pmx_wkup { - pinctrl-names = default; - pinctrl-0 = -twl6030_wkup_pins -; - - twl6030_wkup_pins: pinmux_twl6030_wkup_pins { - pinctrl-single,pins = - 0x14 (PIN_OUTPUT | MUX_MODE2) /* fref_clk0_out.sys_drm_msecure */ -; - }; -}; - omap4_pmx_core { pinctrl-names = default; pinctrl-0 = -twl6030_pins twl6040_pins mcpdm_pins mcbsp1_pins @@ -134,12 +120,6 @@ hsusbb1_pins ; - twl6030_pins: pinmux_twl6030_pins { - pinctrl-single,pins = - 0x15e (WAKEUP_EN | PIN_INPUT_PULLUP | MUX_MODE0) /* sys_nirq1.sys_nirq1 */ -; - }; - twl6040_pins: pinmux_twl6040_pins { pinctrl-single,pins = 0xe0 (PIN_OUTPUT | MUX_MODE3) /* hdq_sio.gpio_127 */ @@ -265,6 +245,7 @@ }; #include twl6030.dtsi +#include twl6030_omap4.dtsi i2c2 { pinctrl-names = default; diff --git a/arch/arm/boot/dts/omap4-sdp.dts b/arch/arm/boot/dts/omap4-sdp.dts index 7951b4e..91030c3 100644 --- a/arch/arm/boot/dts/omap4-sdp.dts +++ b/arch/arm/boot/dts/omap4-sdp.dts @@ -142,23 +142,9 @@ }; }; -omap4_pmx_wkup { - pinctrl-names = default; - pinctrl-0 = -twl6030_wkup_pins -; - - twl6030_wkup_pins: pinmux_twl6030_wkup_pins { - pinctrl-single,pins = - 0x14 (PIN_OUTPUT | MUX_MODE2) /* fref_clk0_out.sys_drm_msecure */ -; - }; -}; - omap4_pmx_core { pinctrl-names = default; pinctrl-0 = -twl6030_pins twl6040_pins mcpdm_pins dmic_pins @@ -193,12 +179,6 @@ ; }; - twl6030_pins: pinmux_twl6030_pins { - pinctrl-single,pins = - 0x15e (WAKEUP_EN | PIN_INPUT_PULLUP | MUX_MODE0) /* sys_nirq1.sys_nirq1 */ -; - }; - twl6040_pins: pinmux_twl6040_pins { pinctrl-single,pins = 0xe0 (PIN_OUTPUT | MUX_MODE3) /* hdq_sio.gpio_127 */ @@ -337,6 +317,7 @@ }; #include twl6030.dtsi +#include twl6030_omap4.dtsi i2c2 { pinctrl-names = default; diff --git a/arch/arm/boot/dts/twl6030_omap4.dtsi b/arch/arm/boot/dts/twl6030_omap4.dtsi new file mode 100644 index 000..a4fa570 --- /dev/null +++ b/arch/arm/boot/dts/twl6030_omap4.dtsi @@ -0,0 +1,38 @@ +/* + * Copyright (C) 2013 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. + */ + +twl { + /* +* On most OMAP4 platforms, the twl6030 IRQ line is connected +* to the SYS_NIRQ1 line on OMAP and the twl6030 MSECURE line is +* connected to the
Re: [Patch V2 3/4] rtc: omap: add rtc wakeup support to alarm events
Hi Andrew, On 7/3/2013 2:17 PM, Hebbar Gururaja wrote: On some platforms (like AM33xx), a special register (RTC_IRQWAKEEN) is available to enable Alarm Wakeup feature. This register needs to be properly handled for the rtcwake to work properly. Platforms using such IP should set ti,am3352-rtc in rtc device dt compatibility node. I just checked that the 1st patch in this series is pulled in and this patch [3/4] is not pulled . If you do not have any comments, can you pull this one also. I have already got Acks from Sekhar Kevin for the same. Thanks regards Gururaja Signed-off-by: Hebbar Gururaja gururaja.heb...@ti.com Acked-by: Kevin Hilman khil...@linaro.org Acked-by: Sekhar Nori nsek...@ti.com Cc: Grant Likely grant.lik...@linaro.org Cc: Rob Herring rob.herr...@calxeda.com Cc: Rob Landley r...@landley.net Cc: Alessandro Zummo a.zu...@towertech.it Cc: rtc-li...@googlegroups.com Cc: devicetree-disc...@lists.ozlabs.org Cc: linux-...@vger.kernel.org --- Changes in V2: - Coding style corrections (use lower case for hex numbers) - use [AM/am]3352 instead of [AM/am]335x to keep the all usages in sync. - Use index defined for struct members so they remain in sync :100644 100644 b47aa41... 5a0f02d... M Documentation/devicetree/bindings/rtc/rtc-omap.txt :100644 100644 761919d... c2e18fe... M drivers/rtc/rtc-omap.c Documentation/devicetree/bindings/rtc/rtc-omap.txt |6 +- drivers/rtc/rtc-omap.c | 60 +--- 2 files changed, 57 insertions(+), 9 deletions(-) diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt b/Documentation/devicetree/bindings/rtc/rtc-omap.txt index b47aa41..5a0f02d 100644 --- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt +++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt @@ -1,7 +1,11 @@ TI Real Time Clock Required properties: -- compatible: ti,da830-rtc +- compatible: + - ti,da830-rtc - for RTC IP used similar to that on DA8xx SoC family. + - ti,am3352-rtc - for RTC IP used similar to that on AM335x SoC family. + This RTC IP has special WAKE-EN Register to enable + Wakeup generation for event Alarm. - reg: Address range of rtc register set - interrupts: rtc timer, alarm interrupts in order - interrupt-parent: phandle for the interrupt controller diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c index 761919d..c2e18fe 100644 --- a/drivers/rtc/rtc-omap.c +++ b/drivers/rtc/rtc-omap.c @@ -72,6 +72,8 @@ #define OMAP_RTC_KICK0_REG0x6c #define OMAP_RTC_KICK1_REG0x70 +#define OMAP_RTC_IRQWAKEEN 0x7c + /* OMAP_RTC_CTRL_REG bit fields: */ #define OMAP_RTC_CTRL_SPLIT (17) #define OMAP_RTC_CTRL_DISABLE (16) @@ -96,12 +98,21 @@ #define OMAP_RTC_INTERRUPTS_IT_ALARM(13) #define OMAP_RTC_INTERRUPTS_IT_TIMER(12) +/* OMAP_RTC_IRQWAKEEN bit fields: */ +#define OMAP_RTC_IRQWAKEEN_ALARM_WAKEEN(11) + /* OMAP_RTC_KICKER values */ #define KICK0_VALUE 0x83e70b13 #define KICK1_VALUE 0x95a4f1e0 #define OMAP_RTC_HAS_KICKER 0x1 +/* + * Few RTC IP revisions has special WAKE-EN Register to enable Wakeup + * generation for event Alarm. + */ +#defineOMAP_RTC_HAS_IRQWAKEEN 0x2 + static void __iomem *rtc_base; #define rtc_read(addr)readb(rtc_base + (addr)) @@ -301,12 +312,18 @@ static struct rtc_class_ops omap_rtc_ops = { static int omap_rtc_alarm; static int omap_rtc_timer; -#defineOMAP_RTC_DATA_DA830_IDX 1 +#defineOMAP_RTC_DATA_AM3352_IDX1 +#defineOMAP_RTC_DATA_DA830_IDX 2 static struct platform_device_id omap_rtc_devtype[] = { { .name = DRIVER_NAME, - }, { + }, + [OMAP_RTC_DATA_AM3352_IDX] = { + .name = am3352-rtc, + .driver_data = OMAP_RTC_HAS_KICKER | OMAP_RTC_HAS_IRQWAKEEN, + }, + [OMAP_RTC_DATA_DA830_IDX] = { .name = da830-rtc, .driver_data = OMAP_RTC_HAS_KICKER, }, @@ -318,6 +335,9 @@ static const struct of_device_id omap_rtc_of_match[] = { { .compatible = ti,da830-rtc, .data = omap_rtc_devtype[OMAP_RTC_DATA_DA830_IDX], }, + { .compatible = ti,am3352-rtc, + .data = omap_rtc_devtype[OMAP_RTC_DATA_AM3352_IDX], + }, {}, }; MODULE_DEVICE_TABLE(of, omap_rtc_of_match); @@ -466,16 +486,28 @@ static u8 irqstat; static int omap_rtc_suspend(struct device *dev) { + u8 irqwake_stat; + struct platform_device *pdev = to_platform_device(dev); + const struct platform_device_id *id_entry = + platform_get_device_id(pdev); + irqstat = rtc_read(OMAP_RTC_INTERRUPTS_REG); /*
Re: [PATCH 1/4] mfd: twl6030-irq: migrate to IRQ threaded handler
On 07/24/2013 03:50 PM, Lee Jones wrote: + if (ret) { + pr_warn(%s: I2C error %d reading PIH ISR\n, __func__, ret); Does the user really care which function we're returning from. Would it be better if you replace '__func__' with the device name? This module hasn't been converted to the device yet:( (I mean interrupt-controller). But I'm thinking about it as the next step :) and then It will be absolutely reasonable change to replace pr_*() with dev_*() and remove __func__. I don't mean anything as compicated as that for 'this' patch. (NB: See my comment in subsequent patches about creating a 'struct twl6030' where you could store 'struct dev'.) In this patch I mean litterally replacing %s: , with tw16030_irq: . Simples. :) Ok. I understand it now - will redo. Now, the pointer on dev (in our case twl-core device) isn't passed in IRQ handler, so It can't be used here. Of course it can be done, but would it make code better? My opinion - no. + if (sts.bytes[2] 0x10) + sts.bytes[2] |= 0x08; - for (i = 0; sts.int_sts; sts.int_sts = 1, i++) { - local_irq_disable(); - if (sts.int_sts 0x1) { - int module_irq = twl6030_irq_base + + for (i = 0; sts.int_sts; sts.int_sts = 1, i++) + if (sts.int_sts 0x1) { I'm a little confused by this. Where does sts.int_sts come from? See my comment above, pls Okay, that's my fault for not understanding unions properly as I've never had to use one, but now I do, thanks. @@ -437,10 +386,13 @@ int twl6030_exit_irq(void) { unregister_pm_notifier(twl6030_irq_pm_notifier_block); - if (twl6030_irq_base) { + if (!twl6030_irq_base) { pr_err(twl6030: can't yet clean up IRQs?\n); return -ENOSYS; } + + free_irq(twl_irq, NULL); + If request_threaded_irq() fails, isn't there a chance that twl6030_irq_base will be allocated, but twl_irq will still be undefined? Yes. A mess is here (historically:), thanks. Will use twl_irq instead of twl6030_irq_base (I did it, actually, in patch [3]:). Yes, I saw it. It would be better if you still fixed up this patch to be correct though. Even if you break it out and add it as [PATCH 1/x]. ok -- 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] ARM: OMAP2+: hwmod: rt address space index for DT
Hi Paul, Benoit, On 7/5/2013 8:43 PM, Afzal Mohammed wrote: Address space is being removed from hwmod database and DT information in reg property is being used. Currently the 0th index of device address space is used to map for register target address. This is not always true, eg. cpgmac has it's sysconfig in second address space. Handle it by specifying index of device address space to be used for register target. As default value of this field would be zero with static initialization, existing behaviour of using first address space for register target while using DT would be kept as such. This series is required to fix the wrong register target address being picked up by hwmod for cpgmac on am335x. Issue happened during last merge window cleanup, where address space was removed from hwmod, and in turn DTS was relied upon for address space(Earlier having the address space in hwmod did not cause any issue as 'ADDR_TYPE_RT' was used to specify proper address space index). Please help this series reach mainline for v3.11 Regards Afzal -- 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 3/4] mfd: twl6030-irq: convert to use linear irq_domain
On 07/24/2013 02:35 PM, Lee Jones wrote: On Tue, 23 Jul 2013, Grygorii Strashko wrote: Since the TWL6030 PMIC is used with OMAP4 SoCs only and OMAP4 legacy boot is dropped there are no needs to allocate the range of IRQ descriptors during system boot to support TWL6030 IRQs. Hence, convert it to use linear irq_domain and move IRQ configuration in .map()/.unmap() callbacks of irq_domain. So, IRQ mapping and descriptors allocation will be performed dynamically basing on DT configuration. The error message will be reported in case if unmapped IRQ is received by TWL6030 (virq==0). Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com --- drivers/mfd/twl6030-irq.c | 114 - 1 file changed, 61 insertions(+), 53 deletions(-) diff --git a/drivers/mfd/twl6030-irq.c b/drivers/mfd/twl6030-irq.c index 790cc28..89f130b 100644 --- a/drivers/mfd/twl6030-irq.c +++ b/drivers/mfd/twl6030-irq.c @@ -138,6 +138,7 @@ static struct notifier_block twl6030_irq_pm_notifier_block = { static irqreturn_t twl6030_irq_thread(int irq, void *data) { int i, ret; + struct irq_domain *irq_domain = (struct irq_domain *)data; union { u8 bytes[4]; u32 int_sts; @@ -161,9 +162,14 @@ static irqreturn_t twl6030_irq_thread(int irq, void *data) for (i = 0; sts.int_sts; sts.int_sts = 1, i++) if (sts.int_sts 0x1) { - int module_irq = twl6030_irq_base + - twl6030_interrupt_mapping[i]; - handle_nested_irq(module_irq); + int module_irq = + irq_find_mapping(irq_domain, +twl6030_interrupt_mapping[i]); + if (module_irq) + handle_nested_irq(module_irq); + else + pr_err(%s: Unmapped PIH ISR %u detected\n, + __func__, i); Same here. Does the user really care which function failed? Please consider replacing with the device name. ok. pr_debug(%s: PIH ISR %u, virq%u\n, __func__, i, module_irq); } @@ -186,19 +192,6 @@ static irqreturn_t twl6030_irq_thread(int irq, void *data) /*--*/ -static inline void activate_irq(int irq) -{ -#ifdef CONFIG_ARM - /* ARM requires an extra step to clear IRQ_NOREQUEST, which it -* sets on behalf of every irq_chip. Also sets IRQ_NOPROBE. -*/ - set_irq_flags(irq, IRQF_VALID); -#else - /* same effect on other architectures */ - irq_set_noprobe(irq); -#endif -} - static int twl6030_irq_set_wake(struct irq_data *d, unsigned int on) { if (on) @@ -308,28 +301,54 @@ int twl6030_mmc_card_detect(struct device *dev, int slot) } EXPORT_SYMBOL(twl6030_mmc_card_detect); +static struct irq_chip twl6030_irq_chip; + +static int twl6030_irq_map(struct irq_domain *d, unsigned int virq, + irq_hw_number_t hwirq) +{ + irq_set_chip_data(virq, twl6030_irq_chip); + irq_set_chip_and_handler(virq, twl6030_irq_chip, handle_simple_irq); + irq_set_nested_thread(virq, true); + +#ifdef CONFIG_ARM + /* +* ARM requires an extra step to clear IRQ_NOREQUEST, which it +* sets on behalf of every irq_chip. Also sets IRQ_NOPROBE. +*/ + set_irq_flags(virq, IRQF_VALID); +#else + /* same effect on other architectures */ + irq_set_noprobe(virq); +#endif + + return 0; +} + +static void twl6030_irq_unmap(struct irq_domain *d, unsigned int virq) +{ +#ifdef CONFIG_ARM + set_irq_flags(virq, 0); +#endif + irq_set_chip_and_handler(virq, NULL, NULL); + irq_set_chip_data(virq, NULL); +} + +static struct irq_domain_ops twl6030_irq_domain_ops = { + .map= twl6030_irq_map, + .unmap = twl6030_irq_unmap, + .xlate = irq_domain_xlate_onetwocell, +}; + int twl6030_init_irq(struct device *dev, int irq_num) { struct device_node *node = dev-of_node; - int nr_irqs, irq_base, irq_end; - static struct irq_chip twl6030_irq_chip; + int nr_irqs; int status; - int i; u8 mask[3]; + struct irq_domain *irq_domain; nr_irqs = TWL6030_NR_IRQS; - irq_base = irq_alloc_descs(-1, 0, nr_irqs, 0); - if (IS_ERR_VALUE(irq_base)) { - dev_err(dev, Fail to allocate IRQ descs\n); - return irq_base; - } - - irq_domain_add_legacy(node, nr_irqs, irq_base, 0, - irq_domain_simple_ops, NULL); - - irq_end = irq_base + nr_irqs; - mask[0] =
Re: [PATCH 4/4] mfd: twl6030-irq: Add interrupt mapping table for the twl6032
On 07/24/2013 02:52 PM, Lee Jones wrote: On Tue, 23 Jul 2013, Grygorii Strashko wrote: From: Oleksandr Dmytryshyn oleksandr.dmytrys...@ti.com This patch adds interrupt mapping table for the twl6032. Repeating the $SUBJECT line is never helpful. Signed-off-by: Oleksandr Dmytryshyn oleksandr.dmytrys...@ti.com Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com --- drivers/mfd/twl6030-irq.c | 49 - 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/drivers/mfd/twl6030-irq.c b/drivers/mfd/twl6030-irq.c index 89f130b..e4df87f 100644 --- a/drivers/mfd/twl6030-irq.c +++ b/drivers/mfd/twl6030-irq.c @@ -41,6 +41,7 @@ #include linux/suspend.h #include linux/of.h #include linux/irqdomain.h +#include linux/of_device.h #include twl-core.h @@ -84,6 +85,36 @@ static int twl6030_interrupt_mapping[24] = { CHARGERFAULT_INTR_OFFSET, /* Bit 22 INT_CHRG*/ RSV_INTR_OFFSET,/* Bit 23 Reserved*/ }; + +static int twl6032_interrupt_mapping[24] = { + PWR_INTR_OFFSET,/* Bit 0PWRON */ + PWR_INTR_OFFSET,/* Bit 1RPWRON */ + PWR_INTR_OFFSET,/* Bit 2SYS_VLOW*/ + RTC_INTR_OFFSET,/* Bit 3RTC_ALARM */ + RTC_INTR_OFFSET,/* Bit 4RTC_PERIOD */ + HOTDIE_INTR_OFFSET, /* Bit 5HOT_DIE */ + SMPSLDO_INTR_OFFSET,/* Bit 6VXXX_SHORT */ + PWR_INTR_OFFSET,/* Bit 7SPDURATION */ + + PWR_INTR_OFFSET,/* Bit 8WATCHDOG*/ + BATDETECT_INTR_OFFSET, /* Bit 9BAT */ + SIMDETECT_INTR_OFFSET, /* Bit 10 SIM */ + MMCDETECT_INTR_OFFSET, /* Bit 11 MMC */ + MADC_INTR_OFFSET, /* Bit 12 GPADC_RT_EOC*/ + MADC_INTR_OFFSET, /* Bit 13 GPADC_SW_EOC*/ + GASGAUGE_INTR_OFFSET, /* Bit 14 CC_EOC */ + GASGAUGE_INTR_OFFSET, /* Bit 15 CC_AUTOCAL */ + + USBOTG_INTR_OFFSET, /* Bit 16 ID_WKUP */ + USBOTG_INTR_OFFSET, /* Bit 17 VBUS_WKUP */ + USBOTG_INTR_OFFSET, /* Bit 18 ID */ + USB_PRES_INTR_OFFSET, /* Bit 19 VBUS*/ + CHARGER_INTR_OFFSET,/* Bit 20 CHRG_CTRL */ + CHARGERFAULT_INTR_OFFSET, /* Bit 21 EXT_CHRG*/ + CHARGERFAULT_INTR_OFFSET, /* Bit 22 INT_CHRG*/ OCD failure. ;) NB: Kidding, you don't have to do anything about this. + RSV_INTR_OFFSET,/* Bit 23 Reserved*/ +}; + /*--*/ static unsigned twl6030_irq_base; @@ -91,6 +122,7 @@ static int twl_irq; static bool twl_irq_wake_enabled; static atomic_t twl6030_wakeirqs = ATOMIC_INIT(0); +static const int *irq_mapping_tbl; What I'd actually like to see is the creation of 'struct twl6030' to keep all your goodies in; irq_domain, irq_mapping_tbl etc and for you to pass that around instead of creating more global variables e.g. via request_threaded_irq(..., void *dev_id) to access the aforementioned information. I can add this as the first patch in series - Is It ok? static int twl6030_irq_pm_notifier(struct notifier_block *notifier, unsigned long pm_event, void *unused) @@ -164,7 +196,7 @@ static irqreturn_t twl6030_irq_thread(int irq, void *data) if (sts.int_sts 0x1) { int module_irq = irq_find_mapping(irq_domain, -twl6030_interrupt_mapping[i]); +irq_mapping_tbl[i]); if (module_irq) handle_nested_irq(module_irq); else @@ -339,6 +371,12 @@ static struct irq_domain_ops twl6030_irq_domain_ops = { .xlate = irq_domain_xlate_onetwocell, }; +static const struct of_device_id twl6030_of_match[] = { + {.compatible = ti,twl6030, twl6030_interrupt_mapping}, + {.compatible = ti,twl6032, twl6032_interrupt_mapping}, + { }, +}; + int twl6030_init_irq(struct device *dev, int irq_num) { struct device_node *node = dev-of_node; @@ -346,6 +384,15 @@ int twl6030_init_irq(struct device *dev, int irq_num) int status; u8 mask[3]; struct irq_domain *irq_domain; + const struct of_device_id *of_id; + + of_id =
Re: OMAP2430 SDP boot broken after Linus' rmk merge
On Tue, Jul 23, 2013 at 10:05:17AM +0100, Rajendra Nayak wrote: On Tuesday 23 July 2013 12:37 PM, Paul Walmsley wrote: Hi Rajendra, On Tue, 23 Jul 2013, Rajendra Nayak wrote: On Tuesday 23 July 2013 01:37 AM, Paul Walmsley wrote: On Mon, 22 Jul 2013, Russell King - ARM Linux wrote: Bear in mind that I'm almost at the point of not boot-testing anything I sent to Linus because of the uselessness of the SDP4430 board now that it's DT only - the only platform which boot-tests anything I send is the 3430LDP board now. If people care about that, maybe they can assist with sorting out the issues I've raised on these lists about the SDP4430 - and why the SDP4430 build remains disabled in my build and boot system. I understand completely... Looking at the boot log, it just stops after uboot hands over control. With the lack of output from the decompressor, it's not possible to tell whether it's a decompressor problem or a kernel problem. I think you need to turn on the LL_DEBUG option, select the appropriate output option, and also get the decompressor to use the kernel's debug io functions to output it's stuff (I think the option is DEBUG_UNCOMPRESS). OK, will dig deeper here at the next opportunity. Paul, I can take a look at the 4430sdp issue. Are you also seeing this also on 2430sdp as the subject says, or was that a typo? Thanks for the offer. The issue that I'm seeing is on the 2430SDP in my testbed. I don't have a 4430SDP, so you might consider touching base with rmk for that one. So I tried commit 'fb2af00' on the 4430SDP and it did boot fine, though I see the below errors. (I am using the mainline bootloaders which do not lock any additional DPLLs like USB) Any update on this? If it's an issue introduced by architectural changes, I'd really like to bisect it down but I don't have a board. Will -- 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: OMAP2430 SDP boot broken after Linus' rmk merge
On Wednesday 24 July 2013 09:56 AM, Will Deacon wrote: On Tue, Jul 23, 2013 at 10:05:17AM +0100, Rajendra Nayak wrote: On Tuesday 23 July 2013 12:37 PM, Paul Walmsley wrote: Hi Rajendra, On Tue, 23 Jul 2013, Rajendra Nayak wrote: On Tuesday 23 July 2013 01:37 AM, Paul Walmsley wrote: On Mon, 22 Jul 2013, Russell King - ARM Linux wrote: Bear in mind that I'm almost at the point of not boot-testing anything I sent to Linus because of the uselessness of the SDP4430 board now that it's DT only - the only platform which boot-tests anything I send is the 3430LDP board now. If people care about that, maybe they can assist with sorting out the issues I've raised on these lists about the SDP4430 - and why the SDP4430 build remains disabled in my build and boot system. I understand completely... Looking at the boot log, it just stops after uboot hands over control. With the lack of output from the decompressor, it's not possible to tell whether it's a decompressor problem or a kernel problem. I think you need to turn on the LL_DEBUG option, select the appropriate output option, and also get the decompressor to use the kernel's debug io functions to output it's stuff (I think the option is DEBUG_UNCOMPRESS). OK, will dig deeper here at the next opportunity. Paul, I can take a look at the 4430sdp issue. Are you also seeing this also on 2430sdp as the subject says, or was that a typo? Thanks for the offer. The issue that I'm seeing is on the 2430SDP in my testbed. I don't have a 4430SDP, so you might consider touching base with rmk for that one. So I tried commit 'fb2af00' on the 4430SDP and it did boot fine, though I see the below errors. (I am using the mainline bootloaders which do not lock any additional DPLLs like USB) Any update on this? If it's an issue introduced by architectural changes, I'd really like to bisect it down but I don't have a board. From the other thread, RMK did manage to get the board booting finally (uImage related issues, low level debug problem) but with DT only supported build, the audio and DSS was not at same state as before(non-DT build). And then Tony pointed the issues to Peter and Tomy to address it further. Russell, Is above right or I am missing something ? Regards, Santosh -- 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: [PATCHv4 00/33] ARM: OMAP: clock conversion to DT
On 07/24/2013 05:16 PM, Roger Quadros wrote: On 07/23/2013 10:19 AM, Tero Kristo wrote: Hi, Changes compared to previous version: - Clock init files moved from mach-omap2/ to drivers/clk/omap/ - AM33xx support added [patches 15-20] - OMAP3 support added [patches 21-29] - DRA7 APLL support added (thanks Keerthy) [patches 30-33] Test branch on top of 3.11-rc1 available here: git://gitorious.org/~kristo/omap-pm/omap-pm-work.git branch: mainline-3.11-rc1-omap-dt-clks Testing done: - boot + suspend tested on OMAP3 beagle C4 (omap3530) - boot + suspend tested on OMAP4 panda ES (omap4460) - boot tested on beagle bone (am335x) A boot test was also executed for DRA7 and OMAP5 on a separate branch (mainline does not have OMAP5 / DRA7 support so far.) Thanks to your hints, I was able to get this to work with 3.11-rc1 on omap5 uevm using the following 3 patches. Please include them in your series. Thanks. Your patches don't apply cleanly to mainline currently, as the omap5 base support is still not merged. I deliberately left out the full omap5 integration because of that. Lets see how and when the integration proceeds, but I can track your patches meanwhile. -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
[PATCH 0/4] Add USB Host support for OMAP5 uevm
Hi, These patches provide USB host support for the OMAP5 uEVM board. They depend on the OMAP clock tree DT data series by Tero Kristo [1] In patch 3 we also provide USB PHY clock for Panda. This should make the clock alias patch [2] redundant and it should be reverted when this series gets in. [1] - http://thread.gmane.org/gmane.linux.ports.arm.omap/101582 [2] - ARM: OMAP2+: Provide alias to USB PHY clock http://git.kernel.org/cgit/linux/kernel/git/tmlind/linux-omap.git/commit/?h=omap-for-v3.11/fixesid=741532c4a995be11815cb72d4d7a48f442a22fea cheers, -roger --- Roger Quadros (4): ARM: OMAP5: hwmod data: Add USB Host and TLL modules ARM: dts: OMAP5: Add 60MHz clock reference to USB Host module ARM: dts: omap4-panda: Provide USB PHY clock ARM: dts: omap5-uevm: Provide USB PHY clock arch/arm/boot/dts/omap4-panda-common.dtsi |8 +- arch/arm/boot/dts/omap5-uevm.dts |8 +- arch/arm/boot/dts/omap5.dtsi |2 + arch/arm/mach-omap2/omap_hwmod_54xx_data.c | 157 4 files changed, 163 insertions(+), 12 deletions(-) -- 1.7.4.1 -- 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 1/4] ARM: OMAP5: hwmod data: Add USB Host and TLL modules
Add hwmod data for High Speed USB host and TLL modules Signed-off-by: Roger Quadros rog...@ti.com --- arch/arm/mach-omap2/omap_hwmod_54xx_data.c | 157 1 files changed, 157 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-omap2/omap_hwmod_54xx_data.c b/arch/arm/mach-omap2/omap_hwmod_54xx_data.c index 3c70f5c..07b2b8b 100644 --- a/arch/arm/mach-omap2/omap_hwmod_54xx_data.c +++ b/arch/arm/mach-omap2/omap_hwmod_54xx_data.c @@ -1433,6 +1433,145 @@ static struct omap_hwmod omap54xx_uart6_hwmod = { }; /* + * 'usb_host_hs' class + * high-speed multi-port usb host controller + */ + +static struct omap_hwmod_class_sysconfig omap54xx_usb_host_hs_sysc = { + .rev_offs = 0x, + .sysc_offs = 0x0010, + .sysc_flags = (SYSC_HAS_MIDLEMODE | SYSC_HAS_RESET_STATUS | + SYSC_HAS_SIDLEMODE | SYSC_HAS_SOFTRESET), + .idlemodes = (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART | + SIDLE_SMART_WKUP | MSTANDBY_FORCE | MSTANDBY_NO | + MSTANDBY_SMART | MSTANDBY_SMART_WKUP), + .sysc_fields= omap_hwmod_sysc_type2, +}; + +static struct omap_hwmod_class omap54xx_usb_host_hs_hwmod_class = { + .name = usb_host_hs, + .sysc = omap54xx_usb_host_hs_sysc, +}; + +static struct omap_hwmod_opt_clk usb_host_hs_opt_clks[] = { + { .role = hsic60m_p2_clk, .clk = usb_host_hs_hsic60m_p2_clk }, + { .role = hsic60m_p3_clk, .clk = usb_host_hs_hsic60m_p3_clk }, + { .role = utmi_p1_clk, .clk = usb_host_hs_utmi_p1_clk }, + { .role = utmi_p2_clk, .clk = usb_host_hs_utmi_p2_clk }, + { .role = utmi_p3_clk, .clk = usb_host_hs_utmi_p3_clk }, + { .role = hsic480m_p1_clk, .clk = usb_host_hs_hsic480m_p1_clk }, + { .role = hsic60m_p1_clk, .clk = usb_host_hs_hsic60m_p1_clk }, + { .role = hsic480m_p3_clk, .clk = usb_host_hs_hsic480m_p3_clk }, + { .role = hsic480m_p2_clk, .clk = usb_host_hs_hsic480m_p2_clk }, +}; + +static struct omap_hwmod omap54xx_usb_host_hs_hwmod = { + .name = usb_host_hs, + .class = omap54xx_usb_host_hs_hwmod_class, + .clkdm_name = l3init_clkdm, + /* +* Errata: USBHOST Configured In Smart-Idle Can Lead To a Deadlock +* id: i660 +* +* Description: +* In the following configuration : +* - USBHOST module is set to smart-idle mode +* - PRCM asserts idle_req to the USBHOST module ( This typically +* happens when the system is going to a low power mode : all ports +* have been suspended, the master part of the USBHOST module has +* entered the standby state, and SW has cut the functional clocks) +* - an USBHOST interrupt occurs before the module is able to answer +* idle_ack, typically a remote wakeup IRQ. +* Then the USB HOST module will enter a deadlock situation where it +* is no more accessible nor functional. +* +* Workaround: +* Don't use smart idle; use only force idle, hence HWMOD_SWSUP_SIDLE +*/ + + /* +* Errata: USB host EHCI may stall when entering smart-standby mode +* Id: i571 +* +* Description: +* When the USBHOST module is set to smart-standby mode, and when it is +* ready to enter the standby state (i.e. all ports are suspended and +* all attached devices are in suspend mode), then it can wrongly assert +* the Mstandby signal too early while there are still some residual OCP +* transactions ongoing. If this condition occurs, the internal state +* machine may go to an undefined state and the USB link may be stuck +* upon the next resume. +* +* Workaround: +* Don't use smart standby; use only force standby, +* hence HWMOD_SWSUP_MSTANDBY +*/ + + /* +* During system boot; If the hwmod framework resets the module +* the module will have smart idle settings; which can lead to deadlock +* (above Errata Id:i660); so, dont reset the module during boot; +* Use HWMOD_INIT_NO_RESET. +*/ + + .flags = HWMOD_SWSUP_SIDLE | HWMOD_SWSUP_MSTANDBY | + HWMOD_INIT_NO_RESET, + .main_clk = l3init_60m_fclk, + .prcm = { + .omap4 = { + .clkctrl_offs = OMAP54XX_CM_L3INIT_USB_HOST_HS_CLKCTRL_OFFSET, + .context_offs = OMAP54XX_RM_L3INIT_USB_HOST_HS_CONTEXT_OFFSET, + .modulemode = MODULEMODE_SWCTRL, + }, + }, + .opt_clks = usb_host_hs_opt_clks, + .opt_clks_cnt = ARRAY_SIZE(usb_host_hs_opt_clks), +}; + +/* + * 'usb_tll_hs' class + * usb_tll_hs module is the adapter on the usb_host_hs ports + */ + +static struct omap_hwmod_class_sysconfig omap54xx_usb_tll_hs_sysc = { + .rev_offs
[PATCH 2/4] ARM: dts: OMAP5: Add 60MHz clock reference to USB Host module
USB Host driver (drivers/mfd/omap-usb-host.c) expects the 60MHz reference clock to be named init_60m_fclk. Provide this information. Signed-off-by: Roger Quadros rog...@ti.com --- arch/arm/boot/dts/omap5.dtsi |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi index bc31022..f9f7f74 100644 --- a/arch/arm/boot/dts/omap5.dtsi +++ b/arch/arm/boot/dts/omap5.dtsi @@ -695,6 +695,8 @@ #address-cells = 1; #size-cells = 1; ranges; + clocks = l3init_60m_fclk; + clock-names = init_60m_fclk; usbhsohci: ohci@4a064800 { compatible = ti,ohci-omap3, usb-ohci; -- 1.7.4.1 -- 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: [PATCHv4 00/33] ARM: OMAP: clock conversion to DT
On 07/24/2013 05:29 PM, Tero Kristo wrote: On 07/24/2013 05:16 PM, Roger Quadros wrote: On 07/23/2013 10:19 AM, Tero Kristo wrote: Hi, Changes compared to previous version: - Clock init files moved from mach-omap2/ to drivers/clk/omap/ - AM33xx support added [patches 15-20] - OMAP3 support added [patches 21-29] - DRA7 APLL support added (thanks Keerthy) [patches 30-33] Test branch on top of 3.11-rc1 available here: git://gitorious.org/~kristo/omap-pm/omap-pm-work.git branch: mainline-3.11-rc1-omap-dt-clks Testing done: - boot + suspend tested on OMAP3 beagle C4 (omap3530) - boot + suspend tested on OMAP4 panda ES (omap4460) - boot tested on beagle bone (am335x) A boot test was also executed for DRA7 and OMAP5 on a separate branch (mainline does not have OMAP5 / DRA7 support so far.) Thanks to your hints, I was able to get this to work with 3.11-rc1 on omap5 uevm using the following 3 patches. Please include them in your series. Thanks. Your patches don't apply cleanly to mainline currently, as the omap5 base support is still not merged. I deliberately left out the full omap5 integration because of that. Lets see how and when the integration proceeds, but I can track your patches meanwhile. Strange, I didn't use any out of tree patch. Just 3.11-rc1 with your 33 patches on top. Are you on 3.11-rc2? cheers, -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: [PATCHv4 00/33] ARM: OMAP: clock conversion to DT
On 07/24/2013 05:34 PM, Roger Quadros wrote: On 07/24/2013 05:29 PM, Tero Kristo wrote: On 07/24/2013 05:16 PM, Roger Quadros wrote: On 07/23/2013 10:19 AM, Tero Kristo wrote: Hi, Changes compared to previous version: - Clock init files moved from mach-omap2/ to drivers/clk/omap/ - AM33xx support added [patches 15-20] - OMAP3 support added [patches 21-29] - DRA7 APLL support added (thanks Keerthy) [patches 30-33] Test branch on top of 3.11-rc1 available here: git://gitorious.org/~kristo/omap-pm/omap-pm-work.git branch: mainline-3.11-rc1-omap-dt-clks Testing done: - boot + suspend tested on OMAP3 beagle C4 (omap3530) - boot + suspend tested on OMAP4 panda ES (omap4460) - boot tested on beagle bone (am335x) A boot test was also executed for DRA7 and OMAP5 on a separate branch (mainline does not have OMAP5 / DRA7 support so far.) Thanks to your hints, I was able to get this to work with 3.11-rc1 on omap5 uevm using the following 3 patches. Please include them in your series. Thanks. Your patches don't apply cleanly to mainline currently, as the omap5 base support is still not merged. I deliberately left out the full omap5 integration because of that. Lets see how and when the integration proceeds, but I can track your patches meanwhile. Strange, I didn't use any out of tree patch. Just 3.11-rc1 with your 33 patches on top. Are you on 3.11-rc2? Oh, so OMAP5 hwmod data was actually merged now, cool. Sorry about the fuss. I can include your patches to the next rev if one is made. -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: OMAP2430 SDP boot broken after Linus' rmk merge
On Wednesday 24 July 2013 10:20 AM, Will Deacon wrote: On Wed, Jul 24, 2013 at 03:17:01PM +0100, Santosh Shilimkar wrote: On Wednesday 24 July 2013 09:56 AM, Will Deacon wrote: On Tue, Jul 23, 2013 at 10:05:17AM +0100, Rajendra Nayak wrote: On Tuesday 23 July 2013 12:37 PM, Paul Walmsley wrote: Hi Rajendra, On Tue, 23 Jul 2013, Rajendra Nayak wrote: On Tuesday 23 July 2013 01:37 AM, Paul Walmsley wrote: On Mon, 22 Jul 2013, Russell King - ARM Linux wrote: Bear in mind that I'm almost at the point of not boot-testing anything I sent to Linus because of the uselessness of the SDP4430 board now that it's DT only - the only platform which boot-tests anything I send is the 3430LDP board now. If people care about that, maybe they can assist with sorting out the issues I've raised on these lists about the SDP4430 - and why the SDP4430 build remains disabled in my build and boot system. I understand completely... Looking at the boot log, it just stops after uboot hands over control. With the lack of output from the decompressor, it's not possible to tell whether it's a decompressor problem or a kernel problem. I think you need to turn on the LL_DEBUG option, select the appropriate output option, and also get the decompressor to use the kernel's debug io functions to output it's stuff (I think the option is DEBUG_UNCOMPRESS). OK, will dig deeper here at the next opportunity. Paul, I can take a look at the 4430sdp issue. Are you also seeing this also on 2430sdp as the subject says, or was that a typo? Thanks for the offer. The issue that I'm seeing is on the 2430SDP in my testbed. I don't have a 4430SDP, so you might consider touching base with rmk for that one. So I tried commit 'fb2af00' on the 4430SDP and it did boot fine, though I see the below errors. (I am using the mainline bootloaders which do not lock any additional DPLLs like USB) Any update on this? If it's an issue introduced by architectural changes, I'd really like to bisect it down but I don't have a board. From the other thread, RMK did manage to get the board booting finally (uImage related issues, low level debug problem) but with DT only supported build, the audio and DSS was not at same state as before(non-DT build). And then Tony pointed the issues to Peter and Tomy to address it further. Russell, Is above right or I am missing something ? I thought that was the 4430 from before the merge window. Ths issue here is the 2430 running mainline, as reported by Paul Actually thread got hijacked a bit when Rajendra replied the OMAP4430 results. I then replied on top of that. Sorry about that Regards, Santosh -- 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: OMAP2430 SDP boot broken after Linus' rmk merge
On Wed, Jul 24, 2013 at 10:17:01AM -0400, Santosh Shilimkar wrote: On Wednesday 24 July 2013 09:56 AM, Will Deacon wrote: On Tue, Jul 23, 2013 at 10:05:17AM +0100, Rajendra Nayak wrote: On Tuesday 23 July 2013 12:37 PM, Paul Walmsley wrote: Hi Rajendra, On Tue, 23 Jul 2013, Rajendra Nayak wrote: On Tuesday 23 July 2013 01:37 AM, Paul Walmsley wrote: On Mon, 22 Jul 2013, Russell King - ARM Linux wrote: Bear in mind that I'm almost at the point of not boot-testing anything I sent to Linus because of the uselessness of the SDP4430 board now that it's DT only - the only platform which boot-tests anything I send is the 3430LDP board now. If people care about that, maybe they can assist with sorting out the issues I've raised on these lists about the SDP4430 - and why the SDP4430 build remains disabled in my build and boot system. I understand completely... Looking at the boot log, it just stops after uboot hands over control. With the lack of output from the decompressor, it's not possible to tell whether it's a decompressor problem or a kernel problem. I think you need to turn on the LL_DEBUG option, select the appropriate output option, and also get the decompressor to use the kernel's debug io functions to output it's stuff (I think the option is DEBUG_UNCOMPRESS). OK, will dig deeper here at the next opportunity. Paul, I can take a look at the 4430sdp issue. Are you also seeing this also on 2430sdp as the subject says, or was that a typo? Thanks for the offer. The issue that I'm seeing is on the 2430SDP in my testbed. I don't have a 4430SDP, so you might consider touching base with rmk for that one. So I tried commit 'fb2af00' on the 4430SDP and it did boot fine, though I see the below errors. (I am using the mainline bootloaders which do not lock any additional DPLLs like USB) Any update on this? If it's an issue introduced by architectural changes, I'd really like to bisect it down but I don't have a board. From the other thread, RMK did manage to get the board booting finally (uImage related issues, low level debug problem) but with DT only supported build, the audio and DSS was not at same state as before(non-DT build). And then Tony pointed the issues to Peter and Tomy to address it further. Russell, Is above right or I am missing something ? Will is referring to the 2430 issues I think; the original topic of this thread. I've not enabled the 4430 again in the build system because I see no point to it - it can't find its rootfs because the device numbering for the MMC/SD cards has reversed itself. Not only does that need the kernel command line changed, but it also needs fixing in the fstab on the SD card rootfs, and quite frankly I really can't be bothered at the moment with this kind of pointless boot breaking churn. I also continue to be disappointed by the lack of things working on the 4430 - it's been a number of years now and _still_ the on-board LCDs do not work. People have tried to blame that on hardware faults and the like, but they're just being idiotic when they say stuff like that. It can't be hardware faults when the kernel supplied with the board is able to make them work to the extent that userspace can play back video on all three output devices simultaneously, without hiccup or any imperfection. I don't know whether it's just that the backlight support isn't working or what - because any information on the 4430 seems to be a tightly controlled secret that only a few select people are permitted to know about. As far as I'm concerned, much of the hardware is a black box to me. And yes, my 4430 has the projector module on it. Never ever seen that work, and no idea how to make it work because there's no information available on the hardware. It's a bit like the useless 3430LDP - though there's information available there's no way to work out which of the many different designs of 3430LDP the one I have ties up with, and I'm pretty sure that all the published revisions of the circuits for the 3430LDP do not match the version I have here - which means when things go wrong, there's no way for me to look at stuff. And no, I don't want another Beagle board or Panda board or whatever, I have those (I think I have one of each), and they've never been powered up because they have no ethernet on them, and I have no USB to ethernet still, partly because I don't really do USB here *at* *all*, so I don't know what works well and what doesn't with Linux. Even if I did provide them with a USB ethernet, I doubt they'd be able to boot a kernel via the network. My experience of USB is hellishly poor - I have an icybox eSATA enclosure which also provides USB. The USB interface on that regularly drops out with errors, timeouts, and even isn't recognised on many occasions. I see slow serial comms with USB serial devices on platforms like the 4430SDP and Dove Cubox - the speed
OMAP4430 SDP boot issues.. (Was Re: OMAP2430 SDP boot broken after Linus' rmk merge)
On Wednesday 24 July 2013 10:52 AM, Russell King - ARM Linux wrote: On Wed, Jul 24, 2013 at 10:17:01AM -0400, Santosh Shilimkar wrote: On Wednesday 24 July 2013 09:56 AM, Will Deacon wrote: On Tue, Jul 23, 2013 at 10:05:17AM +0100, Rajendra Nayak wrote: On Tuesday 23 July 2013 12:37 PM, Paul Walmsley wrote: [..] I don't have a 4430SDP, so you might consider touching base with rmk for that one. So I tried commit 'fb2af00' on the 4430SDP and it did boot fine, though I see the below errors. (I am using the mainline bootloaders which do not lock any additional DPLLs like USB) Any update on this? If it's an issue introduced by architectural changes, I'd really like to bisect it down but I don't have a board. From the other thread, RMK did manage to get the board booting finally (uImage related issues, low level debug problem) but with DT only supported build, the audio and DSS was not at same state as before(non-DT build). And then Tony pointed the issues to Peter and Tomy to address it further. Russell, Is above right or I am missing something ? Will is referring to the 2430 issues I think; the original topic of this thread. We crossed the emails. Am just renaming the thread since below summary is still important for OMAP4430SDP. I've not enabled the 4430 again in the build system because I see no point to it - it can't find its rootfs because the device numbering for the MMC/SD cards has reversed itself. Not only does that need the kernel command line changed, but it also needs fixing in the fstab on the SD card rootfs, and quite frankly I really can't be bothered at the moment with this kind of pointless boot breaking churn. The MMC slot change has also troubled us in past in downstream kernel. Am looping Balaji to see if he can fix it to restore the original ordering. I also continue to be disappointed by the lack of things working on the 4430 - it's been a number of years now and _still_ the on-board LCDs do not work. People have tried to blame that on hardware faults and the like, but they're just being idiotic when they say stuff like that. It can't be hardware faults when the kernel supplied with the board is able to make them work to the extent that userspace can play back video on all three output devices simultaneously, without hiccup or any imperfection. I don't know whether it's just that the backlight support isn't working or what - because any information on the 4430 seems to be a tightly controlled secret that only a few select people are permitted to know about. As far as I'm concerned, much of the hardware is a black box to me. On the display related issues, Tomi and Archit have been sorting out issue but am not sure about the current state. If the pre-built binaries video playback works means your hardware seems to be fine. And yes, my 4430 has the projector module on it. Never ever seen that work, and no idea how to make it work because there's no information available on the hardware. I think you mean the Pico DLP module. There was a downstream driver but am not sure about the state. Archit, Tomi should be able to comment. It's a bit like the useless 3430LDP - though there's information available there's no way to work out which of the many different designs of 3430LDP the one I have ties up with, and I'm pretty sure that all the published revisions of the circuits for the 3430LDP do not match the version I have here - which means when things go wrong, there's no way for me to look at stuff. And no, I don't want another Beagle board or Panda board or whatever, I have those (I think I have one of each), and they've never been powered up because they have no ethernet on them, and I have no USB to ethernet still, partly because I don't really do USB here *at* *all*, so I don't know what works well and what doesn't with Linux. Even if I did provide them with a USB ethernet, I doubt they'd be able to boot a kernel via the network. We discussed the LDP issue in past. That board is almost dead since almost no one from TI is using/having it anymore. My experience of USB is hellishly poor - I have an icybox eSATA enclosure which also provides USB. The USB interface on that regularly drops out with errors, timeouts, and even isn't recognised on many occasions. I see slow serial comms with USB serial devices on platforms like the 4430SDP and Dove Cubox - the speed is very much like a 4800baud modem, even though the serial runs at 115200 baud. No, don't even _think_ about blaming the host, because this happens whether it be a Thinkpad, or the USB hosts in a Thecus N2100. I can believe why this all happens, when you see USB interrupts taking upwards of 3ms to complete: Longest time: 3247506ns Longest irq: 24 Longest handler: usb_hcd_irq+0x0/0x68 is it hardly surprising that USB is soo crap? And the above 3ms is just for handling a USB keyboard - what takes 3ms over servicing a USB keyboard?
[PATCH v3 1/3] Input: omap-keypad: Cleanup - use bitfiled instead of hardcoded values
Use bitfiled instead of hardcoded values to set KBD_CTRL, use BIT macro, remove unused defines. Signed-off-by: Illia Smyrnov illia.smyr...@ti.com Reviewed-by: Felipe Balbi ba...@ti.com --- drivers/input/keyboard/omap4-keypad.c | 25 +++-- 1 files changed, 11 insertions(+), 14 deletions(-) diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c index f4aa53a..c727548 100644 --- a/drivers/input/keyboard/omap4-keypad.c +++ b/drivers/input/keyboard/omap4-keypad.c @@ -53,21 +53,17 @@ #define OMAP4_KBD_FULLCODE63_320x48 /* OMAP4 bit definitions */ -#define OMAP4_DEF_IRQENABLE_EVENTEN(1 0) -#define OMAP4_DEF_IRQENABLE_LONGKEY(1 1) -#define OMAP4_DEF_IRQENABLE_TIMEOUTEN (1 2) -#define OMAP4_DEF_WUP_EVENT_ENA(1 0) -#define OMAP4_DEF_WUP_LONG_KEY_ENA (1 1) -#define OMAP4_DEF_CTRL_NOSOFTMODE (1 1) -#define OMAP4_DEF_CTRLPTVVALUE (1 2) -#define OMAP4_DEF_CTRLPTV (1 1) +#define OMAP4_DEF_IRQENABLE_EVENTENBIT(0) +#define OMAP4_DEF_IRQENABLE_LONGKEYBIT(1) +#define OMAP4_DEF_WUP_EVENT_ENABIT(0) +#define OMAP4_DEF_WUP_LONG_KEY_ENA BIT(1) +#define OMAP4_DEF_CTRL_NOSOFTMODE BIT(1) +#define OMAP4_DEF_CTRL_PTV_SHIFT 2 /* OMAP4 values */ -#define OMAP4_VAL_IRQDISABLE 0x00 -#define OMAP4_VAL_DEBOUNCINGTIME 0x07 -#define OMAP4_VAL_FUNCTIONALCFG0x1E - -#define OMAP4_MASK_IRQSTATUSDISABLE0x +#define OMAP4_VAL_IRQDISABLE 0x0 +#define OMAP4_VAL_DEBOUNCINGTIME 0x7 +#define OMAP4_VAL_PVT 0x7 enum { KBD_REVISION_OMAP4 = 0, @@ -175,7 +171,8 @@ static int omap4_keypad_open(struct input_dev *input) disable_irq(keypad_data-irq); kbd_writel(keypad_data, OMAP4_KBD_CTRL, - OMAP4_VAL_FUNCTIONALCFG); + OMAP4_DEF_CTRL_NOSOFTMODE | + (OMAP4_VAL_PVT OMAP4_DEF_CTRL_PTV_SHIFT)); kbd_writel(keypad_data, OMAP4_KBD_DEBOUNCINGTIME, OMAP4_VAL_DEBOUNCINGTIME); kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS, -- 1.7.0.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 v3 0/3] Input: omap-keypad: Convert to threaded IRQ and cleanup
Replace unclear hardcoded values with bit field, convert to threaded IRQ and clear interrupts when open the keypad. Based on top of v3.11-rc2. Tested on OMAP4 SDP. Illia Smyrnov (3): Input: omap-keypad: Cleanup - use bitfiled instead of hardcoded values Input: omap-keypad: Convert to threaded IRQ Input: omap-keypad: Clear interrupts on open drivers/input/keyboard/omap4-keypad.c | 57 +++-- 1 files changed, 33 insertions(+), 24 deletions(-) -- 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 v3 3/3] Input: omap-keypad: Clear interrupts on open
Clear interrupts when open keypad. According to TRM, the recommended way for keyboard controller initialization is clear the interrupt-status register, then set up certain keyboard events for generating an interrupt request and set up expected source of wake-up event that generates a wake-up request. Signed-off-by: Illia Smyrnov illia.smyr...@ti.com --- drivers/input/keyboard/omap4-keypad.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c index b876a0d..0244262 100644 --- a/drivers/input/keyboard/omap4-keypad.c +++ b/drivers/input/keyboard/omap4-keypad.c @@ -185,8 +185,9 @@ static int omap4_keypad_open(struct input_dev *input) (OMAP4_VAL_PVT OMAP4_DEF_CTRL_PTV_SHIFT)); kbd_writel(keypad_data, OMAP4_KBD_DEBOUNCINGTIME, OMAP4_VAL_DEBOUNCINGTIME); + /* clear pending interrupts */ kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS, - OMAP4_VAL_IRQDISABLE); +kbd_read_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS)); kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQENABLE, OMAP4_DEF_IRQENABLE_EVENTEN | OMAP4_DEF_IRQENABLE_LONGKEY); -- 1.7.0.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 v3 2/3] Input: omap-keypad: Convert to threaded IRQ
Convert to use threaded IRQ. Cc: Felipe Balbi ba...@ti.com Signed-off-by: Illia Smyrnov illia.smyr...@ti.com Reviewed-by: Felipe Balbi ba...@ti.com --- drivers/input/keyboard/omap4-keypad.c | 29 - 1 files changed, 20 insertions(+), 9 deletions(-) diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c index c727548..b876a0d 100644 --- a/drivers/input/keyboard/omap4-keypad.c +++ b/drivers/input/keyboard/omap4-keypad.c @@ -112,8 +112,22 @@ static void kbd_write_irqreg(struct omap4_keypad *keypad_data, } -/* Interrupt handler */ -static irqreturn_t omap4_keypad_interrupt(int irq, void *dev_id) +/* Interrupt handlers */ +static irqreturn_t omap4_keypad_irq_handler(int irq, void *dev_id) +{ + struct omap4_keypad *keypad_data = dev_id; + + if (kbd_read_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS)) { + /* Disable interrupts */ + kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQENABLE, +OMAP4_VAL_IRQDISABLE); + return IRQ_WAKE_THREAD; + } + + return IRQ_NONE; +} + +static irqreturn_t omap4_keypad_irq_thread_fn(int irq, void *dev_id) { struct omap4_keypad *keypad_data = dev_id; struct input_dev *input_dev = keypad_data-input; @@ -121,10 +135,6 @@ static irqreturn_t omap4_keypad_interrupt(int irq, void *dev_id) unsigned int col, row, code, changed; u32 *new_state = (u32 *) key_state; - /* Disable interrupts */ - kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQENABLE, -OMAP4_VAL_IRQDISABLE); - *new_state = kbd_readl(keypad_data, OMAP4_KBD_FULLCODE31_0); *(new_state + 1) = kbd_readl(keypad_data, OMAP4_KBD_FULLCODE63_32); @@ -360,9 +370,10 @@ static int omap4_keypad_probe(struct platform_device *pdev) goto err_free_keymap; } - error = request_irq(keypad_data-irq, omap4_keypad_interrupt, -IRQF_TRIGGER_RISING, -omap4-keypad, keypad_data); + error = request_threaded_irq(keypad_data-irq, omap4_keypad_irq_handler, +omap4_keypad_irq_thread_fn, +IRQF_TRIGGER_RISING, +omap4-keypad, keypad_data); if (error) { dev_err(pdev-dev, failed to register interrupt\n); goto err_free_input; -- 1.7.0.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 v3 3/3] Input: omap-keypad: Clear interrupts on open
Hi, On Wed, Jul 24, 2013 at 06:54:53PM +0300, Illia Smyrnov wrote: Clear interrupts when open keypad. According to TRM, the recommended way for keyboard controller initialization is clear the interrupt-status register, then set up certain keyboard events for generating an interrupt request and set up expected source of wake-up event that generates a wake-up request. Signed-off-by: Illia Smyrnov illia.smyr...@ti.com --- drivers/input/keyboard/omap4-keypad.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c index b876a0d..0244262 100644 --- a/drivers/input/keyboard/omap4-keypad.c +++ b/drivers/input/keyboard/omap4-keypad.c @@ -185,8 +185,9 @@ static int omap4_keypad_open(struct input_dev *input) (OMAP4_VAL_PVT OMAP4_DEF_CTRL_PTV_SHIFT)); kbd_writel(keypad_data, OMAP4_KBD_DEBOUNCINGTIME, OMAP4_VAL_DEBOUNCINGTIME); + /* clear pending interrupts */ kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS, - OMAP4_VAL_IRQDISABLE); + kbd_read_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS)); kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQENABLE, OMAP4_DEF_IRQENABLE_EVENTEN | OMAP4_DEF_IRQENABLE_LONGKEY); Reviewed-by: Felipe Balbi ba...@ti.com -- balbi signature.asc Description: Digital signature
Re: [PATCH 1/3] misc: Add crossbar driver
On 07/22/2013 11:23 AM, Santosh Shilimkar wrote: To summaries it again, what I understood from Sricharan's proposal, - Setup all the routing at cross-bar probe so that kernel continue to work like normal IRQ controller with cross-bar scope vanishes once the routing is done. Cross-bar does this before any of the devices are created. - Something similar needs to happen for DMA lines as well or for any other event routing in future. - Cross-bar callbacks for device drivers for error paths. (Sricharan, you have to drop these because it doesn't bring any functionality and rather can create a side effects of drivers getting polluted.) Ack. The concern raised on above was instead of fixing the routing at DT statically, doing at the driver probes where the loop-up for IRQ or DMA lines should happen in background transparently on drivers call of request_irq/request_dma_channel etc with cross-bar number as an input to it. Though it will be nice to have such feature, it doesn't bring anything special and brings the notion of these APIs which expect that you know what IRQ and DMA lines you want while calling these. Unfortunately, we do have a constraint without allocating dynamic IRQs - what IP instances should hwmod and dts contain? If we go with current series of patches[1] [2] for DRA7 dts which assumes default mapping, hence, uart7-10, GPTimers12-16 dont have default irq - hence they dont exist in dts etc. How would we like to support those with pinctrl approach? Note that mux inputs are pretty much fixed. Its his connection to IRQ controller or DMA controller is what needs to be programmed. So scope is pretty much limited. I felt this requirement is pretty similar to pin-mux and hence thought of it as a viable option. Having said all of above, if there is a better alternative than enhanced pin-mux we surely can do that. We could look at it as a signal mux problem as this thread suggests OR look at it as interrupt distribution problem (which is how it looks like at the face of it). That said, maybe a intermediate pinctrl approach might be more pragmatic and less theoretically flexible. an option might be to statically allocate default number of interrupts to a domain - example: * GIC IRQ 72-78 allotted to UARTs * pinctrl mapping provided for those but only 6 can be used (rest are marked status=disabled as default) at any given time (choice of pinctrl option determines GIC interrupt line to use) * All modules will have a pinctrl definition to have a mapping - to avoid bootloader overriding default cross bar setting in ways un-expected by kernel. Does that sound fair trade off? [1] http://marc.info/?l=linux-omapm=137335524702155w=2 [2] http://marc.info/?l=linux-omapm=137335522802144w=2 -- Regards, Nishanth Menon -- 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/1] arm: dts: dra7-evm: add sd/eMMC node
On 07/23/2013 12:17 PM, Balaji T K wrote: Add micro SD card and eMMC support for dra7-evm Signed-off-by: Rajendra Nayak rna...@ti.com Signed-off-by: Balaji T K balaj...@ti.com --- arch/arm/boot/dts/dra7-evm.dts | 18 ++ 1 files changed, 18 insertions(+), 0 deletions(-) diff --git a/arch/arm/boot/dts/dra7-evm.dts b/arch/arm/boot/dts/dra7-evm.dts index cb0703c..4656ab4 100644 --- a/arch/arm/boot/dts/dra7-evm.dts +++ b/arch/arm/boot/dts/dra7-evm.dts @@ -17,4 +17,22 @@ device_type = memory; reg = 0x8000 0x6000; /* 1536 MB */ }; + + vmmc_fixed: fixedregulator-mmc { + compatible = regulator-fixed; + regulator-name = vmmc_fixed; + regulator-min-microvolt = 330; + regulator-max-microvolt = 330; + }; +}; + +mmc1 { + vmmc-supply = vmmc_fixed; + bus-width = 4; is'nt this going to LDO1 of TPS659039? which allows us to switch it off when MMC card is not needed? +}; + +mmc2 { + vmmc-supply = vmmc_fixed; this is fixes MMC2_3V3 (emmc supply) - EVM_3V3 + bus-width = 8; + ti,non-removable; }; -- Regards, Nishanth Menon -- 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/3] misc: Add crossbar driver
On Wednesday 24 July 2013 12:08 PM, Nishanth Menon wrote: On 07/22/2013 11:23 AM, Santosh Shilimkar wrote: To summaries it again, what I understood from Sricharan's proposal, - Setup all the routing at cross-bar probe so that kernel continue to work like normal IRQ controller with cross-bar scope vanishes once the routing is done. Cross-bar does this before any of the devices are created. - Something similar needs to happen for DMA lines as well or for any other event routing in future. - Cross-bar callbacks for device drivers for error paths. (Sricharan, you have to drop these because it doesn't bring any functionality and rather can create a side effects of drivers getting polluted.) Ack. The concern raised on above was instead of fixing the routing at DT statically, doing at the driver probes where the loop-up for IRQ or DMA lines should happen in background transparently on drivers call of request_irq/request_dma_channel etc with cross-bar number as an input to it. Though it will be nice to have such feature, it doesn't bring anything special and brings the notion of these APIs which expect that you know what IRQ and DMA lines you want while calling these. Unfortunately, we do have a constraint without allocating dynamic IRQs - what IP instances should hwmod and dts contain? If we go with current series of patches[1] [2] for DRA7 dts which assumes default mapping, hence, uart7-10, GPTimers12-16 dont have default irq - hence they dont exist in dts etc. How would we like to support those with pinctrl approach? The whole reason the cross-bar being there because all the peripherals IRQs can not me routed to the IRQ controller. So You pick up a configuration would like to support and stick to it. It will be good to specify all the peripheral devices but tsome of them might not have the IRQ/DMA lines associated with them because of limited slots on IRQ/DMA controller. Note that mux inputs are pretty much fixed. Its his connection to IRQ controller or DMA controller is what needs to be programmed. So scope is pretty much limited. I felt this requirement is pretty similar to pin-mux and hence thought of it as a viable option. Having said all of above, if there is a better alternative than enhanced pin-mux we surely can do that. We could look at it as a signal mux problem as this thread suggests OR look at it as interrupt distribution problem (which is how it looks like at the face of it). Its not interrupt distribution problem but rather setting up the signal routing which otherwise generally hardwired in SOC. Hence the pin-control analogy. Pinmux also lets you set many possible combinations but we pick one for a board design. That said, maybe a intermediate pinctrl approach might be more pragmatic andless theoretically flexible. an option might be to statically allocate default number of interrupts to a domain - example: * GIC IRQ 72-78 allotted to UARTs * pinctrl mapping provided for those but only 6 can be used (rest are marked status=disabled as default) at any given time (choice of pinctrl option determines GIC interrupt line to use) * All modules will have a pinctrl definition to have a mapping - to avoid bootloader overriding default cross bar setting in ways un-expected by kernel. Does that sound fair trade off? This sounds better. That way we can get all the devices in the DT at least. Regards, Santosh -- 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/3] misc: Add crossbar driver
On 07/24/2013 11:38 AM, Santosh Shilimkar wrote: On Wednesday 24 July 2013 12:08 PM, Nishanth Menon wrote: That said, maybe a intermediate pinctrl approach might be more pragmatic and less theoretically flexible. an option might be to statically allocate default number of interrupts to a domain - example: * GIC IRQ 72-78 allotted to UARTs * pinctrl mapping provided for those but only 6 can be used (rest are marked status=disabled as default) at any given time (choice of pinctrl option determines GIC interrupt line to use) * All modules will have a pinctrl definition to have a mapping - to avoid bootloader overriding default cross bar setting in ways un-expected by kernel. Does that sound fair trade off? This sounds better. That way we can get all the devices in the DT at least. Fair enough - if Linus and Tony are still ok with this approach to the problem, seeing a patch series with the effect would be beneficial. -- Regards, Nishanth Menon -- 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
ARM: dts: omap3-beagle-xm: fix compatible property
Hey Guys, While working on the device tree for the xM i noticed this little error on the compatible property... Thoughts? From af8e3119ae2ee29bcf739d56168d6aaa3a2c8306 Mon Sep 17 00:00:00 2001 From: Robert Nelson robertcnel...@gmail.com Date: Wed, 24 Jul 2013 12:02:11 -0500 Subject: [PATCH] ARM: dts: omap3-beagle-xm: fix compatible property Signed-off-by: Robert Nelson robertcnel...@gmail.com --- arch/arm/boot/dts/omap3-beagle-xm.dts |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/omap3-beagle-xm.dts b/arch/arm/boot/dts/omap3-beagle-xm.dts index afdb164..0c514dc 100644 --- a/arch/arm/boot/dts/omap3-beagle-xm.dts +++ b/arch/arm/boot/dts/omap3-beagle-xm.dts @@ -11,7 +11,7 @@ / { model = TI OMAP3 BeagleBoard xM; - compatible = ti,omap3-beagle-xm, ti,omap3-beagle, ti,omap3; + compatible = ti,omap3-beagle-xm, ti,omap3-beagle, ti,omap3; cpus { cpu@0 { -- 1.7.10.4 -- Robert Nelson http://www.rcn-ee.com/ -- 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] mtd: nand: omap2: Fix compilation warning
Hi Olof, fb1585bc13b (mtd: nand: omap2: clean-up BCHx_HW and BCHx_SW ECC configurations in device_probe) introduced a warning when the new option is disabled, i.e. with omap2plus_defconfig: drivers/mtd/nand/omap2.c:1075:13: warning: 'omap3_enable_hwecc_bch' defined but not used [-Wunused-function] Fix this by rescoping the ifdef. Also remove a redudant #endif/#ifdef pair. Signed-off-by: Olof Johansson o...@lixom.net --- Thanks much.. But just to remind, this commit fb1585b in linux-next needs to be reverted. This patch is leftover by mistake because other dependent patches of this series were dropped. This will not only break omap2-nand driver, but might also break the build. http://lists.infradead.org/pipermail/linux-mtd/2013-July/047441.html Please let me know, if something needs to be done from my-side to get this reverted. I have already re-sent a updated version of this series with fixes copying devicetree-discuss. http://lists.infradead.org/pipermail/linux-mtd/2013-July/047530.html with regards, pekon -- 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: OMAP4430 SDP boot issues.. (Was Re: OMAP2430 SDP boot broken after Linus' rmk merge)
On Wednesday 24 July 2013 01:23 PM, Russell King - ARM Linux wrote: On Wed, Jul 24, 2013 at 11:40:33AM -0400, Santosh Shilimkar wrote: On Wednesday 24 July 2013 10:52 AM, Russell King - ARM Linux wrote: I also continue to be disappointed by the lack of things working on the 4430 - it's been a number of years now and _still_ the on-board LCDs do not work. People have tried to blame that on hardware faults and the like, but they're just being idiotic when they say stuff like that. It can't be hardware faults when the kernel supplied with the board is able to make them work to the extent that userspace can play back video on all three output devices simultaneously, without hiccup or any imperfection. I don't know whether it's just that the backlight support isn't working or what - because any information on the 4430 seems to be a tightly controlled secret that only a few select people are permitted to know about. As far as I'm concerned, much of the hardware is a black box to me. On the display related issues, Tomi and Archit have been sorting out issue but am not sure about the current state. If the pre-built binaries video playback works means your hardware seems to be fine. Just let me be absolutely crystal clear about this: the on-board LCDs have _never_ worked with a mainline kernel, but they did work with the kernel which originally came with the board with the original 4430 SoC. I can right now put the original 4430 SoC back on the board and boot using the kernel which is still on the SD card and they will work. Even with the original 4430 SoC and mainline kernels, they have never worked. I tried it back and forth several times, but even with this, the explanation was always your hardware must be faulty. And then the wrong voltage levels was found which was preventing the LCD modules from even being recognised... but still, no sign of life on the LCD panels. It may be that there's some error in the kernel configuration I'm building. I don't know, because I have _no_ idea what hardware is involved in bringing the LCDs online. Like I said above, the 4430SDP board is just a black box. I am totally reliant on others telling me what the right config options are supposed to be and fixing problems when things don't work. With the lack of information on this board, all I can do when things don't work is whinge and moan at people. I am powerless to debug the problems myself, even if they're a simple misconfiguration. Is there information available on the 4430SDP? Is there any reason I can't have access to it? I haven't come across the information which is available on the web. Let me check if there are schematics which i can send across to you. Regards, Santosh -- 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 01/15] drivers: phy: add generic PHY framework
On Tuesday 23 July 2013, Tomasz Figa wrote: On Tuesday 23 of July 2013 17:14:20 Alan Stern wrote: On Tue, 23 Jul 2013, Tomasz Figa wrote: Where would you want to have those phy_address arrays stored? There are no board files when booting with DT. Not even saying that you don't need to use any hacky schemes like this when you have DT that nicely specifies relations between devices. If everybody agrees DT has a nice scheme for specifying relations between devices, why not use that same scheme in the PHY core? It is already used, for cases when consumer device has a DT node attached. In non-DT case this kind lookup translates loosely to something that is being done in regulator framework - you can't bind devices by pointers, because you don't have those pointers, so you need to use device names. Sorry for jumping in to the middle of the discussion, but why does a *new* framework even bother defining an interface for board files? Can't we just drop any interfaces for platform data passing in the phy framework and put the burden of adding those to anyone who actually needs them? All the platforms we are concerned with here (exynos and omap, plus new platforms) can be booted using DT anyway. Arnd -- 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/3] misc: Add crossbar driver
On Wednesday 24 July 2013 10:17 PM, Nishanth Menon wrote: On 07/24/2013 11:38 AM, Santosh Shilimkar wrote: On Wednesday 24 July 2013 12:08 PM, Nishanth Menon wrote: That said, maybe a intermediate pinctrl approach might be more pragmatic and less theoretically flexible. an option might be to statically allocate default number of interrupts to a domain - example: * GIC IRQ 72-78 allotted to UARTs * pinctrl mapping provided for those but only 6 can be used (rest are marked status=disabled as default) at any given time (choice of pinctrl option determines GIC interrupt line to use) * All modules will have a pinctrl definition to have a mapping - to avoid bootloader overriding default cross bar setting in ways un-expected by kernel. Does that sound fair trade off? This sounds better. That way we can get all the devices in the DT at least. Fair enough - if Linus and Tony are still ok with this approach to the problem, seeing a patch series with the effect would be beneficial. Ok, i will use this idea of certain number interrupts to groups. Yes on DRA7XX, we have about 160 gic lines and 320 irq crossbar device inputs contending for it. 1:2 and fully arbitrary. But will we be really exhausting them ? Regards, Sricharan -- 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: v3.11-rc1 USB regressions
Hi, On Tue, Jul 16, 2013 at 08:57:10PM +0300, Aaro Koskinen wrote: On Tue, Jul 16, 2013 at 08:33:06AM +0200, Daniel Mack wrote: fe4cb0912f8e737f8e4b8b38b9e692f8062f5423 usb: musb: gadget: remove hcd initialization (Reverting this fixes error cdc_ether: probe of 4-1:1.0 failed with error -110 seen on the host side.) Which role does your musb-based hardware have in this case, and which MUSB_* config symbols do you set? You should have USB_MUSB_GADGET. Here are full details: Kernel v3.11-rc1 and the following USB fixes: ARM: omap2: fix musb usage for n8x0 musb: omap: Fix: pass all the resources to musb core [...] When I revert fe4cb0912f8e737f8e4b8b38b9e692f8062f5423 and 8b125df5b24cfb0ec7fa1971e343cc0badc1827d, it works like before (3.10): I'm now running -rc2 with above fixes and reverts (the only way to get USB working). I'm seeing an additional issue, the following crash happens always on N900 when doing poweroff: [ 33.198699] musb-hdrc musb-hdrc.0.auto: remove, state 4 [ 33.206817] usb usb1: USB disconnect, device number 1 [ 33.215332] musb-hdrc musb-hdrc.0.auto: USB bus 1 deregistered [ 33.225128] Unable to handle kernel NULL pointer dereference at virtual address 0028 [ 33.236083] pgd = c0914000 [ 33.241149] [0028] *pgd=8efc8831, *pte=, *ppte= [ 33.250213] Internal error: Oops: 17 [#1] ARM [ 33.257141] Modules linked in: usb_f_ecm g_ether u_ether libcomposite configfs omapfb cfbcopyarea cfbimgblt cfbfillrect panel_acx565akm [ 33.275146] CPU: 0 PID: 760 Comm: poweroff Not tainted 3.11.0-rc2-n9xx #1 [ 33.284881] task: cefb8680 ti: c08c6000 task.ti: c08c6000 [ 33.293151] PC is at usb_remove_hcd+0x10/0x178 [ 33.300384] LR is at musb_gadget_stop+0x184/0x1a8 [ 33.307830] pc : [c02499d0]lr : [c0261688]psr: 2013 [ 33.307830] sp : c08c7da0 ip : c08c7dc0 fp : c08c7dbc [ 33.324920] r10: r9 : c08c6000 r8 : 2013 [ 33.332916] r7 : cf814010 r6 : cf814fa0 r5 : cf814fa0 r4 : [ 33.342315] r3 : c0262b80 r2 : 0001 r1 : c0d9edd0 r0 : [ 33.351715] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user [ 33.361846] Control: 10c5387d Table: 80914019 DAC: 0015 [ 33.370483] Process poweroff (pid: 760, stack limit = 0xc08c6238) [ 33.379486] Stack: (0xc08c7da0 to 0xc08c8000) [ 33.386627] 7da0: c0262b80 cf814fa0 c08c7de4 c08c7dc0 c0261688 c02499cc [ 33.397918] 7dc0: c0261504 cef2d200 cf814fa0 cf93fc44 c0d9d698 c0c8bb9c c08c7dfc c08c7de8 [ 33.409149] 7de0: c0262dd8 c0261510 bf03f3a4 cef2d200 c08c7e14 c08c7e00 c0263894 c0262d60 [ 33.420440] 7e00: cf814010 cf93fc10 c08c7e24 c08c7e18 c0261c80 c026380c c08c7e3c c08c7e28 [ 33.431732] 7e20: c025b2a4 c0261c78 cf93fc1c cf93fc10 c08c7e4c c08c7e40 c01e88ec c025b27c [ 33.443023] 7e40: c08c7e74 c08c7e50 c01e55d0 c01e88d8 4321fedc 4321fedc 00070784 [ 33.454315] 7e60: fee1dead c000e848 c08c7e84 c08c7e78 c00553c4 c01e55ac c08c7fa4 c08c7e88 [ 33.465606] 7e80: c0055658 c0055394 0020 cef3add0 c0c72f6c [ 33.476898] 7ea0: c08c7ecc c08c7eb0 c017a914 c01791d0 0001 0001 c0c74078 [ 33.488159] 7ec0: c08c7efc c08c7ed0 c00524b8 c017a8e8 0001 0007 c0337640 c0c5d9e4 [ 33.499328] 7ee0: b6e5d670 c08c7fb0 b6f28000 c08c7fac c08c7f00 c0008514 c033764c [ 33.510498] 7f00: c08c7f04 c08c7f04 7fff cefc1c00 0001 0010 [ 33.521667] 7f20: 0002 c08c7f00 cefc1c00 cef4da00 cefc1c00 [ 33.532836] 7f40: cf803858 cf8037d4 c00e0528 c08c7f84 c08c7f60 c00ea404 c00d1bb0 [ 33.544067] 7f60: 0004 0055 0024 c000e848 c08c7fa4 c08c7f88 [ 33.555297] 7f80: c00e0698 c00ea3a4 0001 0004 0055 0058 c08c7fa8 [ 33.566528] 7fa0: c000e6a0 c00554a4 0001 0004 fee1dead 28121969 4321fedc 00070784 [ 33.577789] 7fc0: 0001 0004 0055 0058 b6f28000 [ 33.589080] 7fe0: b6e5d740 be88ccc0 000605f8 b6e5d758 6010 fee1dead 8fefe821 8fefec21 [ 33.600402] Backtrace: [ 33.605621] [c02499c0] (usb_remove_hcd+0x0/0x178) from [c0261688] (musb_gadget_stop+0x184/0x1a8) [ 33.620666] r5:cf814fa0 r4: [ 33.627166] [c0261504] (musb_gadget_stop+0x0/0x1a8) from [c0262dd8] (usb_gadget_remove_driver+0x84/0xa4) [ 33.643005] r8:c0c8bb9c r7:c0d9d698 r6:cf93fc44 r5:cf814fa0 r4:cef2d200 r3:c0261504 [ 33.656890] [c0262d54] (usb_gadget_remove_driver+0x0/0xa4) from [c0263894] (usb_del_gadget_udc+0x94/0xd4) [ 33.673065] r4:cef2d200 r3:bf03f3a4 [ 33.679718] [c0263800] (usb_del_gadget_udc+0x0/0xd4) from [c0261c80] (musb_gadget_cleanup+0x14/0x18) [ 33.695465] r5:cf93fc10 r4:cf814010 [ 33.702178] [c0261c6c] (musb_gadget_cleanup+0x0/0x18) from [c025b2a4] (musb_shutdown+0x34/0x94) [ 33.717651] [c025b270] (musb_shutdown+0x0/0x94) from
Re: [PATCH 1/3] misc: Add crossbar driver
On 07/24/2013 01:43 PM, Sricharan R wrote: On Wednesday 24 July 2013 10:17 PM, Nishanth Menon wrote: On 07/24/2013 11:38 AM, Santosh Shilimkar wrote: On Wednesday 24 July 2013 12:08 PM, Nishanth Menon wrote: That said, maybe a intermediate pinctrl approach might be more pragmatic and less theoretically flexible. an option might be to statically allocate default number of interrupts to a domain - example: * GIC IRQ 72-78 allotted to UARTs * pinctrl mapping provided for those but only 6 can be used (rest are marked status=disabled as default) at any given time (choice of pinctrl option determines GIC interrupt line to use) * All modules will have a pinctrl definition to have a mapping - to avoid bootloader overriding default cross bar setting in ways un-expected by kernel. Does that sound fair trade off? This sounds better. That way we can get all the devices in the DT at least. Fair enough - if Linus and Tony are still ok with this approach to the problem, seeing a patch series with the effect would be beneficial. Ok, i will use this idea of certain number interrupts to groups. Yes on DRA7XX, we have about 160 gic lines and 320 irq crossbar device inputs contending for it. 1:2 and fully arbitrary. But will we be really exhausting them ? Depends on how we allocate :). The default arbitary allocation can be made more logical in your series ofcourse :). -- Regards, Nishanth Menon -- 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] dma: edma: add device_slave_caps() support
On 07/24/2013 03:40 AM, Lars-Peter Clausen wrote: On 07/24/2013 10:28 AM, Fernandes, Joel wrote: On Jul 24, 2013, at 3:23 AM, Lars-Peter Clausen l...@metafoo.de wrote: On 07/24/2013 10:11 AM, Joel Fernandes wrote: On 07/24/2013 03:03 AM, Lars-Peter Clausen wrote: On 07/23/2013 06:43 PM, Joel Fernandes wrote: Implement device_slave_caps(). EDMA has a limited number of slots. Slave drivers such as omap_hsmmc will query the driver to make sure they don't pass in more than these many scatter segments. Signed-off-by: Joel Fernandes jo...@ti.com --- Vinod, or Dan- If this patch looks ok, can you please merge in for -rc cycle? This patch is required to fix MMC support on AM33xx. This patch is blocking 3 other patches which fix various MMC things. Thanks! Notes: (1) this approach is temporary and only for -rc cycle to fix MMC on AM335x. It will be replace by the RFC series in future kernels: http://www.spinics.net/lists/arm-kernel/msg260094.html (2) Patch depends Vinod's patch at: http://permalink.gmane.org/gmane.linux.kernel/1525112 drivers/dma/edma.c |9 + 1 file changed, 9 insertions(+) diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c index 7222cbe..81d5429 100644 --- a/drivers/dma/edma.c +++ b/drivers/dma/edma.c @@ -517,6 +517,14 @@ static void edma_issue_pending(struct dma_chan *chan) spin_unlock_irqrestore(echan-vchan.lock, flags); } +static inline int edma_slave_caps(struct dma_chan *chan, +struct dma_slave_caps *caps) +{ +caps-max_sg_nr = MAX_NR_SG; Hm, what about the other fields? Other fields are unused, the max segment size is supposed to be calculated given the address width and burst size. Since these can't be provided to get_caps, I have left it out for now. See: https://lkml.org/lkml/2013/3/6/464 The PL330 driver is similar in this regard, the maximum segment size also depends on address width and burst width. What I did for the get_slave_caps implementation is to set it to the minimum maximum size. E.g. in you case that should be SZ_64K - 1 (burstsize and addrwidth both set to 1). So you're setting max to minimum maximum size? Isn't that like telling the driver that its segments can't be bigger than this... Unless I'm missing something.. Yes. This is a limitation of the current slave_caps API. The maximum needs to be the maximum for all possible configurations. A specific configuration may allow a larger maximum. So we maybe have to extend the API to be able to query the limits for a certain configuration. Not sure what the best way would be to do that, either adding a config parameter to get_slave_caps or to break it into two functions like you proposed one for the static capabilities and one for the sg limits. I am OK with either approach as long as a decision can be made quickly by maintainers. Right now lot of back and forth has happened and 3 different versions of the same thing have been posted since January. Since this is such a trivial change, it doesn't make sense to spend so much time on it IMO The sad part is though this change is trivial, other drivers such as MMC are broken and cannot be enabled due to this. We cannot afford to leave them broken. If Vinod is not available, can Dan please respond on how to proceed on this? We really need this trivial change to go into this -rc cycle and not delay it by another kernel release. Thank you. -Joel -- 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/3] misc: Add crossbar driver
On Wednesday 24 July 2013 02:51 PM, Nishanth Menon wrote: On 07/24/2013 01:43 PM, Sricharan R wrote: On Wednesday 24 July 2013 10:17 PM, Nishanth Menon wrote: On 07/24/2013 11:38 AM, Santosh Shilimkar wrote: On Wednesday 24 July 2013 12:08 PM, Nishanth Menon wrote: That said, maybe a intermediate pinctrl approach might be more pragmatic and less theoretically flexible. an option might be to statically allocate default number of interrupts to a domain - example: * GIC IRQ 72-78 allotted to UARTs * pinctrl mapping provided for those but only 6 can be used (rest are marked status=disabled as default) at any given time (choice of pinctrl option determines GIC interrupt line to use) * All modules will have a pinctrl definition to have a mapping - to avoid bootloader overriding default cross bar setting in ways un-expected by kernel. Does that sound fair trade off? This sounds better. That way we can get all the devices in the DT at least. Fair enough - if Linus and Tony are still ok with this approach to the problem, seeing a patch series with the effect would be beneficial. Ok, i will use this idea of certain number interrupts to groups. Yes on DRA7XX, we have about 160 gic lines and 320 irq crossbar device inputs contending for it. 1:2 and fully arbitrary. But will we be really exhausting them ? Depends on how we allocate :). The default arbitary allocation can be made more logical in your series ofcourse :). I would just most logical peripherals rather than providing every single IP connected to cross bar. Otherwise we will end up wth hwmod like scenario where now started removing the unused stuff because of maintenance and loc issues ;-) -- 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: v3.11-rc1 USB regressions
Hi Aaro, On 24.07.2013 20:51, Aaro Koskinen wrote: On Tue, Jul 16, 2013 at 08:57:10PM +0300, Aaro Koskinen wrote: Here are full details: Kernel v3.11-rc1 and the following USB fixes: ARM: omap2: fix musb usage for n8x0 musb: omap: Fix: pass all the resources to musb core [...] When I revert fe4cb0912f8e737f8e4b8b38b9e692f8062f5423 and 8b125df5b24cfb0ec7fa1971e343cc0badc1827d, it works like before (3.10): I'm now running -rc2 with above fixes and reverts (the only way to get USB working). I'm seeing an additional issue, the following crash happens always on N900 when doing poweroff: Yes, with the mentioned patches reverted, musb_to_hcd() will return a faulty pointer. You can't easily revert them unfortunately. Your platform needs a real fix, I just have trouble understanding why a removed usb_add_hcd() would make the gadget code fail. Sorry for the trouble, but I don't currently have a board with musb in gadget mode to reproduce this issue. Thanks, Daniel -- 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] dma: edma: add device_slave_caps() support
On Wed, Jul 24, 2013 at 01:55:24PM -0500, Joel Fernandes wrote: On 07/24/2013 03:40 AM, Lars-Peter Clausen wrote: On 07/24/2013 10:28 AM, Fernandes, Joel wrote: Vinod, or Dan- If this patch looks ok, can you please merge in for -rc cycle? This patch is required to fix MMC support on AM33xx. This patch is blocking 3 other patches which fix various MMC things. Thanks! Sorry I was travelling so not realy on top of email for last few days... Now I am not sure we can send this to -rc. If it were just this one, we could have pushed but it also depends on a new API which is sitting in my -next. I am not super comfortable to send that to Linus for the -rcs. Sure, he would scream at me! Also another point worth considering is the approach Russell suggested, I havent gotten a chance to dig deeper but if I understood it correctly then programming the device_dma_parameters should be the right thing to do. Again I need to look deeper and esp wrt edma ~Vinod -- 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] dma: edma: add device_slave_caps() support
On 07/24/2013 08:55 PM, Joel Fernandes wrote: On 07/24/2013 03:40 AM, Lars-Peter Clausen wrote: On 07/24/2013 10:28 AM, Fernandes, Joel wrote: On Jul 24, 2013, at 3:23 AM, Lars-Peter Clausen l...@metafoo.de wrote: On 07/24/2013 10:11 AM, Joel Fernandes wrote: On 07/24/2013 03:03 AM, Lars-Peter Clausen wrote: On 07/23/2013 06:43 PM, Joel Fernandes wrote: Implement device_slave_caps(). EDMA has a limited number of slots. Slave drivers such as omap_hsmmc will query the driver to make sure they don't pass in more than these many scatter segments. Signed-off-by: Joel Fernandes jo...@ti.com --- Vinod, or Dan- If this patch looks ok, can you please merge in for -rc cycle? This patch is required to fix MMC support on AM33xx. This patch is blocking 3 other patches which fix various MMC things. Thanks! Notes: (1) this approach is temporary and only for -rc cycle to fix MMC on AM335x. It will be replace by the RFC series in future kernels: http://www.spinics.net/lists/arm-kernel/msg260094.html (2) Patch depends Vinod's patch at: http://permalink.gmane.org/gmane.linux.kernel/1525112 drivers/dma/edma.c |9 + 1 file changed, 9 insertions(+) diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c index 7222cbe..81d5429 100644 --- a/drivers/dma/edma.c +++ b/drivers/dma/edma.c @@ -517,6 +517,14 @@ static void edma_issue_pending(struct dma_chan *chan) spin_unlock_irqrestore(echan-vchan.lock, flags); } +static inline int edma_slave_caps(struct dma_chan *chan, +struct dma_slave_caps *caps) +{ +caps-max_sg_nr = MAX_NR_SG; Hm, what about the other fields? Other fields are unused, the max segment size is supposed to be calculated given the address width and burst size. Since these can't be provided to get_caps, I have left it out for now. See: https://lkml.org/lkml/2013/3/6/464 The PL330 driver is similar in this regard, the maximum segment size also depends on address width and burst width. What I did for the get_slave_caps implementation is to set it to the minimum maximum size. E.g. in you case that should be SZ_64K - 1 (burstsize and addrwidth both set to 1). So you're setting max to minimum maximum size? Isn't that like telling the driver that its segments can't be bigger than this... Unless I'm missing something.. Yes. This is a limitation of the current slave_caps API. The maximum needs to be the maximum for all possible configurations. A specific configuration may allow a larger maximum. So we maybe have to extend the API to be able to query the limits for a certain configuration. Not sure what the best way would be to do that, either adding a config parameter to get_slave_caps or to break it into two functions like you proposed one for the static capabilities and one for the sg limits. I am OK with either approach as long as a decision can be made quickly by maintainers. Right now lot of back and forth has happened and 3 different versions of the same thing have been posted since January. Since this is such a trivial change, it doesn't make sense to spend so much time on it IMO The sad part is though this change is trivial, other drivers such as MMC are broken and cannot be enabled due to this. We cannot afford to leave them broken. Well this is a new API, so it is kind of expected that there is some back and forth and that there will be a few revisions. If Vinod is not available, can Dan please respond on how to proceed on this? We really need this trivial change to go into this -rc cycle and not delay it by another kernel release. Thank you. This is not something you'd merge for rc3 or even later. If the MMC driver does not work without this I guess it never worked, so strictly speaking there is no regression and it is just a new feature. - 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] dma: edma: add device_slave_caps() support
On 07/24/2013 01:33 PM, Vinod Koul wrote: On Wed, Jul 24, 2013 at 01:55:24PM -0500, Joel Fernandes wrote: On 07/24/2013 03:40 AM, Lars-Peter Clausen wrote: On 07/24/2013 10:28 AM, Fernandes, Joel wrote: Vinod, or Dan- If this patch looks ok, can you please merge in for -rc cycle? This patch is required to fix MMC support on AM33xx. This patch is blocking 3 other patches which fix various MMC things. Thanks! Sorry I was travelling so not realy on top of email for last few days... Now I am not sure we can send this to -rc. OK. If it were just this one, we could have pushed but it also depends on a new API which is sitting in my -next. I am not super comfortable to send that to Linus for the -rcs. Sure, he would scream at me! OK. Also another point worth considering is the approach Russell suggested, I havent gotten a chance to dig deeper but if I understood it correctly then programming the device_dma_parameters should be the right thing to do. Again I need to look deeper and esp wrt edma OK. I have some patches sitting in my tree too that I'm working on. With that I don't need to know about maximum number of allowed segments and can send along any number of segment. I will rework them and post them. fwiw, I will also implement caps API incase like Lars did populating the other fields though these will not be unused. For segment size, at this time I don't know any driver that uses it other than davinci-pcm. For this reason the calculations can be done as Lars suggested (for minimum of maximum). Do you know in advance if you're going to amend to drop segment size if we go with what Russell suggested, or are you going to leave the seg-size in the caps API anyway. Thanks, -Joel -- 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: v3.11-rc1 USB regressions
Hi, On Wed, Jul 24, 2013 at 09:04:28PM +0200, Daniel Mack wrote: On 24.07.2013 20:51, Aaro Koskinen wrote: When I revert fe4cb0912f8e737f8e4b8b38b9e692f8062f5423 and 8b125df5b24cfb0ec7fa1971e343cc0badc1827d, it works like before (3.10): I'm now running -rc2 with above fixes and reverts (the only way to get USB working). I'm seeing an additional issue, the following crash happens always on N900 when doing poweroff: Yes, with the mentioned patches reverted, musb_to_hcd() will return a faulty pointer. You can't easily revert them unfortunately. Your platform needs a real fix, I just have trouble understanding why a removed usb_add_hcd() would make the gadget code fail. Sorry for the trouble, but I don't currently have a board with musb in gadget mode to reproduce this issue. If you have any ideas what to look for, I can maybe try to debug this issue. Below are some logs with additional DEBUGs enabled. Broken: [4.111480] udc musb-hdrc.0.auto: registering UDC driver [g_ether] [4.136962] using random self ethernet address [4.144134] using random host ethernet address [4.151000] g_ether gadget: adding config #1 'CDC Ethernet (ECM)'/bf03f2c0 [4.160552] g_ether gadget: adding 'cdc_ethernet'/cef92e40 to config 'CDC Ethernet (ECM)'/bf03f2c0 [4.186828] usb0: HOST MAC f6:4a:86:02:5d:ec [4.193939] usb0: MAC 26:e9:7c:49:4a:2e [4.200256] g_ether gadget: CDC Ethernet: dual speed IN/ep1in OUT/ep1out NOTIFY/ep2in [4.210906] g_ether gadget: cfg 1/bf03f2c0 speeds: high full [4.219207] g_ether gadget: interface 0 = cdc_ethernet/cef92e40 [4.227874] g_ether gadget: interface 1 = cdc_ethernet/cef92e40 [4.236480] g_ether gadget: Ethernet Gadget, version: Memorial Day 2008 [4.245635] g_ether gadget: g_ether ready [4.261627] twl4030_usb twl4030_usb: twl4030_phy_resume [4.281433] usb0: eth_open [4.560974] musb-hdrc musb-hdrc.0.auto: ** IRQ peripheral usb000d tx0001 rx [4.570983] musb-hdrc musb-hdrc.0.auto: == DevCtl=99, int_usb=0xd [4.579681] musb-hdrc musb-hdrc.0.auto: SUSPEND (b_idle) devctl 99 [4.588378] musb-hdrc musb-hdrc.0.auto: BUS RESET as b_peripheral [4.596984] musb-hdrc musb-hdrc.0.auto: RX ep0 fifo fa0ab020 count 8 buf c0c55df0 [4.607055] musb-hdrc musb-hdrc.0.auto: TX ep0 fifo fa0ab020 count 18 buf cef1a000 [4.617248] musb-hdrc musb-hdrc.0.auto: ** IRQ peripheral usb0008 tx0001 rx [4.627044] musb-hdrc musb-hdrc.0.auto: == DevCtl=99, int_usb=0x8 [4.728790] musb-hdrc musb-hdrc.0.auto: ** IRQ peripheral usb000c tx0001 rx [4.738616] musb-hdrc musb-hdrc.0.auto: == DevCtl=99, int_usb=0xc [4.747222] musb-hdrc musb-hdrc.0.auto: BUS RESET as b_peripheral [4.755645] musb-hdrc musb-hdrc.0.auto: RX ep0 fifo fa0ab020 count 8 buf c0c55df0 [4.765533] musb-hdrc musb-hdrc.0.auto: ** IRQ peripheral usb0008 tx0001 rx [4.775207] musb-hdrc musb-hdrc.0.auto: == DevCtl=99, int_usb=0x8 [4.784637] musb-hdrc musb-hdrc.0.auto: ** IRQ peripheral usb0008 tx0001 rx [4.794281] musb-hdrc musb-hdrc.0.auto: == DevCtl=99, int_usb=0x8 [4.802703] musb-hdrc musb-hdrc.0.auto: RX ep0 fifo fa0ab020 count 8 buf c0c55df0 [4.812500] musb-hdrc musb-hdrc.0.auto: TX ep0 fifo fa0ab020 count 18 buf cef1a000 [4.822509] musb-hdrc musb-hdrc.0.auto: ** IRQ peripheral usb0008 tx0001 rx [4.832153] musb-hdrc musb-hdrc.0.auto: == DevCtl=99, int_usb=0x8 [4.840576] musb-hdrc musb-hdrc.0.auto: RX ep0 fifo fa0ab020 count 8 buf c0c55dd0 [4.850372] musb-hdrc musb-hdrc.0.auto: TX ep0 fifo fa0ab020 count 9 buf cef1a000 [4.860198] musb-hdrc musb-hdrc.0.auto: ** IRQ peripheral usb0008 tx0001 rx [4.869781] musb-hdrc musb-hdrc.0.auto: == DevCtl=99, int_usb=0x8 [4.878112] musb-hdrc musb-hdrc.0.auto: RX ep0 fifo fa0ab020 count 8 buf c0c55ce8 [4.887847] musb-hdrc musb-hdrc.0.auto: TX ep0 fifo fa0ab020 count 64 buf cef1a000 [4.897674] musb-hdrc musb-hdrc.0.auto: ** IRQ peripheral usb0008 tx0001 rx [4.907257] musb-hdrc musb-hdrc.0.auto: == DevCtl=99, int_usb=0x8 [4.915588] musb-hdrc musb-hdrc.0.auto: TX ep0 fifo fa0ab020 count 24 buf cef1a040 [4.925506] musb-hdrc musb-hdrc.0.auto: ** IRQ peripheral usb0008 tx0001 rx [4.935089] musb-hdrc musb-hdrc.0.auto: == DevCtl=99, int_usb=0x8 [4.943450] musb-hdrc musb-hdrc.0.auto: RX ep0 fifo fa0ab020 count 8 buf c0c55ce8 [4.953247] musb-hdrc musb-hdrc.0.auto: TX ep0 fifo fa0ab020 count 4 buf cef1a000 [4.963012] musb-hdrc musb-hdrc.0.auto: ** IRQ peripheral usb0008 tx0001 rx [4.972595] musb-hdrc musb-hdrc.0.auto: == DevCtl=99, int_usb=0x8 [4.980926] musb-hdrc musb-hdrc.0.auto: RX ep0 fifo fa0ab020 count 8 buf c0c55ce8 [4.990692] musb-hdrc musb-hdrc.0.auto: TX ep0 fifo fa0ab020 count 32 buf cef1a000 [5.000549] musb-hdrc musb-hdrc.0.auto: ** IRQ peripheral usb0008 tx0001 rx [5.010101] musb-hdrc musb-hdrc.0.auto: == DevCtl=99, int_usb=0x8 [
Re: [net-next PATCH v2 1/1] drivers: net: cpsw: add support to show hw stats via ethtool
From: Mugunthan V N mugunthan...@ti.com Date: Tue, 23 Jul 2013 15:38:17 +0530 Add support to show CPSW hardware statistics to user via ethtool so user can find if there were any error reported by hardware or the system is over loaded duing high data rate transfer. Signed-off-by: Mugunthan V N mugunthan...@ti.com --- Changes from initial version * Change hw_stats variable from struct cpsw_hw_stats __iomem * to u8 __iomem *, so that accessing hw_stats doesn't require type casting. This looks a lot better, applied, thanks. -- 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] dma: edma: add device_slave_caps() support
Sent from my iPhone On Jul 24, 2013, at 2:15 PM, Lars-Peter Clausen l...@metafoo.de wrote: On 07/24/2013 08:55 PM, Joel Fernandes wrote: On 07/24/2013 03:40 AM, Lars-Peter Clausen wrote: On 07/24/2013 10:28 AM, Fernandes, Joel wrote: On Jul 24, 2013, at 3:23 AM, Lars-Peter Clausen l...@metafoo.de wrote: On 07/24/2013 10:11 AM, Joel Fernandes wrote: On 07/24/2013 03:03 AM, Lars-Peter Clausen wrote: On 07/23/2013 06:43 PM, Joel Fernandes wrote: Implement device_slave_caps(). EDMA has a limited number of slots. Slave drivers such as omap_hsmmc will query the driver to make sure they don't pass in more than these many scatter segments. Signed-off-by: Joel Fernandes jo...@ti.com --- Vinod, or Dan- If this patch looks ok, can you please merge in for -rc cycle? This patch is required to fix MMC support on AM33xx. This patch is blocking 3 other patches which fix various MMC things. Thanks! Notes: (1) this approach is temporary and only for -rc cycle to fix MMC on AM335x. It will be replace by the RFC series in future kernels: http://www.spinics.net/lists/arm-kernel/msg260094.html (2) Patch depends Vinod's patch at: http://permalink.gmane.org/gmane.linux.kernel/1525112 drivers/dma/edma.c |9 + 1 file changed, 9 insertions(+) diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c index 7222cbe..81d5429 100644 --- a/drivers/dma/edma.c +++ b/drivers/dma/edma.c @@ -517,6 +517,14 @@ static void edma_issue_pending(struct dma_chan *chan) spin_unlock_irqrestore(echan-vchan.lock, flags); } +static inline int edma_slave_caps(struct dma_chan *chan, +struct dma_slave_caps *caps) +{ +caps-max_sg_nr = MAX_NR_SG; Hm, what about the other fields? Other fields are unused, the max segment size is supposed to be calculated given the address width and burst size. Since these can't be provided to get_caps, I have left it out for now. See: https://lkml.org/lkml/2013/3/6/464 The PL330 driver is similar in this regard, the maximum segment size also depends on address width and burst width. What I did for the get_slave_caps implementation is to set it to the minimum maximum size. E.g. in you case that should be SZ_64K - 1 (burstsize and addrwidth both set to 1). So you're setting max to minimum maximum size? Isn't that like telling the driver that its segments can't be bigger than this... Unless I'm missing something.. Yes. This is a limitation of the current slave_caps API. The maximum needs to be the maximum for all possible configurations. A specific configuration may allow a larger maximum. So we maybe have to extend the API to be able to query the limits for a certain configuration. Not sure what the best way would be to do that, either adding a config parameter to get_slave_caps or to break it into two functions like you proposed one for the static capabilities and one for the sg limits. I am OK with either approach as long as a decision can be made quickly by maintainers. Right now lot of back and forth has happened and 3 different versions of the same thing have been posted since January. Since this is such a trivial change, it doesn't make sense to spend so much time on it IMO The sad part is though this change is trivial, other drivers such as MMC are broken and cannot be enabled due to this. We cannot afford to leave them broken. Well this is a new API, so it is kind of expected that there is some back and forth and that there will be a few revisions. Sure. Only thing bothered me is it is a few lines and is just API semantics, nothing functional really. The MMC dt patches were posted but not applied. I said regression because the dt was agreed for -rc cycle but only thing missing is this trivial api stuff so possibly counting that as a regression fixes MMC altogether. 6 months for trivial change blocking an otherwise fully working driver is too much. I am speaking collectively for all of us, not me or anyone in particular. Anyway looks like MMC is not going anywhere till then. If Vinod is not available, can Dan please respond on how to proceed on this? We really need this trivial change to go into this -rc cycle and not delay it by another kernel release. Thank you. This is not something you'd merge for rc3 or even later. If the MMC driver does not work without this I guess it never worked, so strictly speaking there is no regression and it is just a new feature. Agreed. -Joel - 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] of: provide of_platform_unpopulate()
On Mon, 22 Jul 2013 16:16:07 -0500, Rob Herring robherri...@gmail.com wrote: On 07/21/2013 06:44 PM, Grant Likely wrote: On Sun, Jul 21, 2013 at 9:48 PM, Rob Herring robherri...@gmail.com wrote: On 07/21/2013 09:42 AM, Rob Herring wrote: On 07/19/2013 01:14 PM, Sebastian Andrzej Siewior wrote: So I called of_platform_populate() on a device to get each child device probed and on rmmod and I need to reverse its doing. After a quick grep I did what others did as well and rmmod ended in: | Unable to handle kernel NULL pointer dereference at virtual address 0018 | PC is at release_resource+0x18/0x80 | Process rmmod (pid: 2005, stack limit = 0xedc30238) | [c003add0] (release_resource+0x18/0x80) from [c0300e08] (platform_device_del+0x78/0xac) | [c0300e08] (platform_device_del+0x78/0xac) from [c0301358] (platform_device_unregister+0xc/0x18) The problem is that platform_device_del() releases each ressource in its tree. This does not work on platform_devices created by OF becuase they were never added via insert_resource(). As a consequence old-parent in __release_resource() is NULL and we explode while accessing -child. So I either I do something completly wrong _or_ nobody here tested the rmmod path of their driver. Wouldn't the correct fix be to call insert_resource somehow? The problem I have is that while of_platform_populate is all about parsing the DT and creating devices, the removal side has nothing to do with DT. So this should not be in the DT code. I think the core device code should be able to handle removal if the device creation side is done correctly. It looks to me like of_device_add either needs to call platform_device_add rather than device_add. I think the device name setting in platform_device_add should be a nop. If not, a check that the name is already set could be added. BTW, it looks like Grant has attempted this already: Yup, things broke badly. Unfortunately the of_platform_device and platform_device history doesn't treat resources in the same way. I would like to merge the code, but I haven't been able to figure out a clean way to do it. Looks like we do need the unpopulate function. Was there more breakage than imx6 and amba devices? Your first version had a fallback case for powerpc. Couldn't we do just allow that for more than just powerpc? I'd much rather see some work-around within the core DT code with a warning to prevent more proliferation than putting this into drivers. It's tricky stuff. I've not figured out a solution I'm happy with. Trying to figure out when to apply a work around is hard because the resource reservation makes assumptions about the memory range layout that doesn't match the assumptions made by device tree code. One /possible/ option is to not add the resources to the devices at all when the device is registered and instead resolve them right at bind time. Jean Christophe proposed doing this already to solve a different problem; obtaining resources that require other drivers to be probed first. If the resources are resolved at .probe() time, then the resource registration problem should also go away. The downside to that approach is that it makes each deferred probe more expensive; potentially a *lot* more expensive depending on how much work the xlate functions have to do. It would be worth prototyping though to see how well it works. g. -- 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