Re: [PATCH 0/3] ARM: dts: Enable EDMA, MMC and SPI on AM33XX for v3.13
On 09/11/2013 12:18 AM, Koen Kooi wrote: Op 10 sep. 2013, om 22:14 heeft Joel Fernandes jo...@ti.com het volgende geschreven: On 09/10/2013 02:39 PM, Koen Kooi wrote: Op 10 sep. 2013, om 21:24 heeft Joel Fernandes jo...@ti.com het volgende geschreven: Here are last few patches required to add EDMA and MMC/SPI support for AM33xx. Now that all dependent DMA patches and fixes are in linux next or mainline, except for [1] which should go in for 3.12 -rc cycle, it is safe to enable MMC and SPI support and this patch series enables it. These are originally Matt Porter's patches with changes to make it work with recent kernels, addition of irq, memory resources and enable other extra properties. These patches should cleanly apply on master branch after Koen's patch [2] for basic BBB DT support is applied. MMC support is enabled for: Beaglebone, AM335x EVM and EVM-SK boards. MMC support for BBB is intentionally not added due to custom fixes and other patches that are in Koen's tree and which will be separately submitted by him. Correct, but your patches for MMC support on BBW are missing the card detect entries to make it hotplug work. I thought it was determined that this would be submitted by you separately after rebasing as we discussed [1] and [2]. I have no problem submitting that, I just think it's weird that the patch you submitted contains a known broken version for BBW. There's nothing broken about $subject series. Please don't confuse maintainers by using wrong words like that. This series is perfectly OK as such to be merged. Further, I am puzzled by all this noise because card-detect additions were initially agreed to be posted separately by you along with other custom DTS for BBW MMC. Its obvious I wouldn't squash patches that we _agreed_ you would send out- and that are especially additions than any real fixes. Hopefully this makes it clear, if you need any help please let me know. Thanks! -Joel [1] https://lkml.org/lkml/2013/9/6/183 [2] http://marc.info/?l=linux-omapm=137879246709612w=2 Regards, -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 0/3] ARM: dts: Enable EDMA, MMC and SPI on AM33XX for v3.13
Op 11 sep. 2013, om 08:00 heeft Joel Fernandes jo...@ti.com het volgende geschreven: On 09/11/2013 12:18 AM, Koen Kooi wrote: Op 10 sep. 2013, om 22:14 heeft Joel Fernandes jo...@ti.com het volgende geschreven: On 09/10/2013 02:39 PM, Koen Kooi wrote: Op 10 sep. 2013, om 21:24 heeft Joel Fernandes jo...@ti.com het volgende geschreven: Here are last few patches required to add EDMA and MMC/SPI support for AM33xx. Now that all dependent DMA patches and fixes are in linux next or mainline, except for [1] which should go in for 3.12 -rc cycle, it is safe to enable MMC and SPI support and this patch series enables it. These are originally Matt Porter's patches with changes to make it work with recent kernels, addition of irq, memory resources and enable other extra properties. These patches should cleanly apply on master branch after Koen's patch [2] for basic BBB DT support is applied. MMC support is enabled for: Beaglebone, AM335x EVM and EVM-SK boards. MMC support for BBB is intentionally not added due to custom fixes and other patches that are in Koen's tree and which will be separately submitted by him. Correct, but your patches for MMC support on BBW are missing the card detect entries to make it hotplug work. I thought it was determined that this would be submitted by you separately after rebasing as we discussed [1] and [2]. I have no problem submitting that, I just think it's weird that the patch you submitted contains a known broken version for BBW. There's nothing broken about $subject series. Please don't confuse maintainers by using wrong words like that. This series is perfectly OK as such to be merged. But plugging in an SD card doesn't work in this series, I'd call that broken. Further, I am puzzled by all this noise because card-detect additions were initially agreed to be posted separately by you along with other custom DTS for BBW MMC. I agreed to post the mmc fixes needed for BBW and BBB, so I assumed you'd drop all beaglebone entries from your series. Its obvious I wouldn't squash patches that we _agreed_ you would send out- and that are especially additions than any real fixes. Hopefully this makes it clear, if you need any help please let me know. Right, so now your patch series will add a half-assed DT entry which only works if the uSD card is present at boot. Anyway, fixup series underway already Thanks! -Joel [1] https://lkml.org/lkml/2013/9/6/183 [2] http://marc.info/?l=linux-omapm=137879246709612w=2 Regards, -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
[PATCH 0/2] ARM: dts: Beaglebone MMC fixes
Here are two patches to fix MMC on beaglebone, one fixes card detect on BBW, the other adds the eMMC entry for BBB and its fixed regulator. This series depends on: http://comments.gmane.org/gmane.linux.kernel.stable/63648 https://lkml.org/lkml/2013/9/10/454 http://comments.gmane.org/gmane.linux.kernel.mmc/22381 Or as git-cherry would put it: [koen@rrMBP patches]$ git cherry -v + c8436cd15251ea42c7f2decd1cc972de536a7573 ARM: OMAP2+: am335x-bone*: add DT for BeagleBone Black + 34596cb58346612101b168b34b7ee75b8b0e9500 ARM: EDMA: Fix clearing of unused list for DT DMA resources + af7f273dfbeeec7669c01a81b8c02f4af2fcf1b9 ARM: dts: add AM33XX EDMA support + a85f9acc8f48a7cb1a07fb93fcba7bd43aa9c853 ARM: dts: add AM33XX SPI DMA support + 72a42fa0b42051251c554d9f75f42b3e9bc6ec8a ARM: dts: add AM33XX MMC support and documentation + 93205a3735613d674b209c3c75ad3e0a7d6be136 arm: bone: dts: add CD for mmc1 + b1bd98705a2da3d02f197d29ca040008ad8c662c am335x-boneblack: add eMMC DT entry Also available as a git branch at https://github.com/koenkooi/linux/commits/mainline --- Alexander Holler (1): arm: bone: dts: add CD for mmc1 Koen Kooi (1): am335x-boneblack: add eMMC DT entry arch/arm/boot/dts/am335x-bone-common.dtsi | 21 + arch/arm/boot/dts/am335x-bone.dts | 4 arch/arm/boot/dts/am335x-boneblack.dts| 15 +++ 3 files changed, 36 insertions(+), 4 deletions(-) -- 1.8.2.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/2] arm: bone: dts: add CD for mmc1
From: Alexander Holler hol...@ahsoftware.de
This enables the use of MMC cards even when no card was inserted at boot.
Signed-off-by: Alexander Holler hol...@ahsoftware.de
Signed-off-by: Koen Kooi k...@dominion.thruhere.net
---
arch/arm/boot/dts/am335x-bone-common.dtsi | 14 ++
arch/arm/boot/dts/am335x-bone.dts | 4
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/arch/arm/boot/dts/am335x-bone-common.dtsi
b/arch/arm/boot/dts/am335x-bone-common.dtsi
index 2f66ded..ced256c 100644
--- a/arch/arm/boot/dts/am335x-bone-common.dtsi
+++ b/arch/arm/boot/dts/am335x-bone-common.dtsi
@@ -107,6 +107,11 @@
0x14c (PIN_INPUT_PULLDOWN | MUX_MODE7)
;
};
+ mmc1_pins: pinmux_mmc1_pins {
+ pinctrl-single,pins =
+ 0x160 (PIN_INPUT | MUX_MODE7) /* GPIO0_6 */
+ ;
+ };
};
ocp {
@@ -260,3 +265,12 @@
pinctrl-0 = davinci_mdio_default;
pinctrl-1 = davinci_mdio_sleep;
};
+
+mmc1 {
+ status = okay;
+ vmmc-supply = ldo3_reg;
+ pinctrl-names = default;
+ pinctrl-0 = mmc1_pins;
+ cd-gpios = gpio0 6 GPIO_ACTIVE_HIGH;
+ cd-inverted;
+};
diff --git a/arch/arm/boot/dts/am335x-bone.dts
b/arch/arm/boot/dts/am335x-bone.dts
index d5f43fe..d34469c 100644
--- a/arch/arm/boot/dts/am335x-bone.dts
+++ b/arch/arm/boot/dts/am335x-bone.dts
@@ -16,7 +16,3 @@
regulator-always-on;
};
-mmc1 {
- status = okay;
- vmmc-supply = ldo3_reg;
-};
--
1.8.2.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 2/2] am335x-boneblack: add eMMC DT entry
Signed-off-by: Koen Kooi k...@dominion.thruhere.net
---
arch/arm/boot/dts/am335x-bone-common.dtsi | 7 +++
arch/arm/boot/dts/am335x-boneblack.dts| 15 +++
2 files changed, 22 insertions(+)
diff --git a/arch/arm/boot/dts/am335x-bone-common.dtsi
b/arch/arm/boot/dts/am335x-bone-common.dtsi
index ced256c..9c61fef 100644
--- a/arch/arm/boot/dts/am335x-bone-common.dtsi
+++ b/arch/arm/boot/dts/am335x-bone-common.dtsi
@@ -241,6 +241,13 @@
regulator-always-on;
};
};
+
+ vmmcsd_fixed: fixedregulator@0 {
+ compatible = regulator-fixed;
+ regulator-name = vmmcsd_fixed;
+ regulator-min-microvolt = 330;
+ regulator-max-microvolt = 330;
+ };
};
cpsw_emac0 {
diff --git a/arch/arm/boot/dts/am335x-boneblack.dts
b/arch/arm/boot/dts/am335x-boneblack.dts
index 197cadf..c09947a 100644
--- a/arch/arm/boot/dts/am335x-boneblack.dts
+++ b/arch/arm/boot/dts/am335x-boneblack.dts
@@ -15,3 +15,18 @@
regulator-max-microvolt = 180;
regulator-always-on;
};
+
+mmc1 {
+ vmmc-supply = vmmcsd_fixed;
+};
+
+mmc2 {
+ vmmc-supply = vmmcsd_fixed;
+ pinctrl-names = default;
+ pinctrl-0 = emmc_pins;
+ bus-width = 8;
+ ti,non-removable;
+ status = okay;
+ ti,vcc-aux-disable-is-sleep;
+};
+
--
1.8.2.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: commit 5fe212364 causes division by zero with large bauds
Hi Felipe,
Thanks for finding this issue. Indeed, there is a bug on 3M+ baud
rates. First patch is close to a complete fix, but still contains
div-by-zero issue. Here is my version:
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 816d1a2..808a880 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -240,8 +240,8 @@ serial_omap_baud_is_mode16(struct uart_port *port,
unsigned int baud)
{
unsigned int n13 = port-uartclk / (13 * baud);
unsigned int n16 = port-uartclk / (16 * baud);
- int baudAbsDiff13 = baud - (port-uartclk / (13 * n13));
- int baudAbsDiff16 = baud - (port-uartclk / (16 * n16));
+ int baudAbsDiff13 = n13 ? (baud - (port-uartclk / (13 *
n13))) : INT_MAX;
+ int baudAbsDiff16 = n16 ? (baud - (port-uartclk / (16 *
n16))) : INT_MAX;
if(baudAbsDiff13 0)
baudAbsDiff13 = -baudAbsDiff13;
if(baudAbsDiff16 0)
With 48MHz UART clock, it will give
300: divisor = 12307 (13), real rate 300 (0.00%)
600: divisor = 6153 (13), real rate 600 (0.00%)
1200: divisor = 3076 (13), real rate 1200 (0.00%)
2400: divisor = 1538 (13), real rate 2400 (0.00%)
4800: divisor = 625 (16), real rate 4800 (0.00%)
9600: divisor = 384 (13), real rate 9615 (0.156250%)
14400: divisor = 256 (13), real rate 14423 (0.159722%)
19200: divisor = 192 (13), real rate 19230 (0.156250%)
28800: divisor = 128 (13), real rate 28846 (0.159722%)
38400: divisor = 96 (13), real rate 38461 (0.158854%)
57600: divisor = 64 (13), real rate 57692 (0.159722%)
115200: divisor = 32 (13), real rate 115384 (0.159722%)
230400: divisor = 16 (13), real rate 230769 (0.160156%)
460800: divisor = 8 (13), real rate 461538 (0.160156%)
921600: divisor = 4 (13), real rate 923076 (0.160156%)
100: divisor = 3 (16), real rate 100 (0.00%)
1843200: divisor = 2 (13), real rate 1846153 (0.160211%)
300: divisor = 1 (16), real rate 300 (0.00%)
3686400: divisor = 1 (13), real rate 3692307 (0.160238%)
Thanks,
Alexey
On Tue, Sep 10, 2013 at 10:09 PM, Felipe Balbi ba...@ti.com wrote:
Hi Alexey,
your commit 5fe212364 causes division by zero with any baud rate larger
than 3 Mbaud (IP supports up to 3686400).
Maybe this patch is all we need to get it fixed ? (untested)
diff --git a/drivers/tty/serial/omap-serial.c
b/drivers/tty/serial/omap-serial.c
index 816d1a2..b50327f 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -240,8 +240,14 @@ serial_omap_baud_is_mode16(struct uart_port *port,
unsigned int baud)
{
unsigned int n13 = port-uartclk / (13 * baud);
unsigned int n16 = port-uartclk / (16 * baud);
- int baudAbsDiff13 = baud - (port-uartclk / (13 * n13));
- int baudAbsDiff16 = baud - (port-uartclk / (16 * n16));
+ int baudAbsDiff13 = 0;
+ int baudAbsDiff16 = 0;
+
+ if (n13)
+ baudAbsDiff13 = baud - (port-uartclk / (13 * n13));
+ if (n16)
+ baudAbsDiff16 = baud - (port-uartclk / (16 * n16));
+
if(baudAbsDiff13 0)
baudAbsDiff13 = -baudAbsDiff13;
if(baudAbsDiff16 0)
Another possibility would be to convert this into a lookup table (also
untested):
diff --git a/drivers/tty/serial/omap-serial.c
b/drivers/tty/serial/omap-serial.c
index 816d1a2..942d68e 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -224,6 +224,48 @@ static void serial_omap_enable_wakeup(struct
uart_omap_port *up, bool enable)
pdata-enable_wakeup(up-dev, enable);
}
+struct uart_omap_config {
+ unsigned int baud;
+ unsigned int oversampling;
+ unsigned int divisor;
+};
+
+static struct uart_omap_config omap_port_configs[] = {
+ { .baud = 300, .oversampling = 16, .divisor = 1, },
+ { .baud = 300, .oversampling = 16, .divisor = 1, },
+ { .baud = 600, .oversampling = 16, .divisor = 5000, },
+ { .baud = 1200, .oversampling = 16, .divisor = 2500, },
+ { .baud = 2400, .oversampling = 16, .divisor = 1250, },
+ { .baud = 4800, .oversampling = 16, .divisor = 625, },
+ { .baud = 9600, .oversampling = 16, .divisor = 312, },
+ { .baud = 14400,.oversampling = 16, .divisor = 208, },
+ { .baud = 19200,.oversampling = 16, .divisor = 156, },
+ { .baud = 28800,.oversampling = 16, .divisor = 704, },
+ { .baud = 38400,.oversampling = 16, .divisor = 78, },
+ { .baud = 57600,.oversampling = 16, .divisor = 52, },
+ { .baud = 115200, .oversampling = 16, .divisor = 26, },
+ { .baud = 230400, .oversampling = 16, .divisor = 13, },
+ { .baud = 460800, .oversampling = 13, .divisor = 8, },
+ { .baud
Re: [PATCH] RFC: interrupt consistency check for OF GPIO IRQs
Am 10.09.2013 17:00, schrieb Joel Fernandes: I think your initial patch is much better than fixing up DT but then I may be missing other problems with your patch that Linus's patch addresses. The initial patch had the problem that it not only did introduce irq-mappings for only those gpios which are marked as IRQs, but it requested those gpios too and preconfigured them And that breaks every driver which uses gpios for IRQs. To summarize what happens if a driver uses a gpio as irq: gpio_request() // This works only if the gpio was not requested before gpio_direction_input() gpio_to_irq() // This needs an irq-mapping request_threaded_irq() So I would suggest multiple steps to change that: 1. Create a mapping for every gpio found in DT (or all gpios if no DT is used). I think that is what Linus patch does (sorry, I haven't really followed this thread and didn't look in deep at the patch). 2. Implement gpio_request_for_irq() This would just be a small macro for gpio_request(); gpio_direction_input() 3. Change all drivers which do use gpio_to_irq() to use gpio_request_for_irq() instead of gpio_request() and gpio_direction_input(). This will end up with a big series of more or less trivial patches (I count 677 occurences of gpio_to_irq) and might be splitted. 4. request gpios and set them to input when a gpio is marked as an IRQ in the DT and DT is used. Change gpio_rquest_for_irq() to a NOP if DT is used or leave it as it is for the none-dt case. Regards, Alexander Holler -- 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] RFC: interrupt consistency check for OF GPIO IRQs
Am 11.09.2013 09:05, schrieb Alexander Holler: Am 10.09.2013 17:00, schrieb Joel Fernandes: I think your initial patch is much better than fixing up DT but then I may be missing other problems with your patch that Linus's patch addresses. The initial patch had the problem that it not only did introduce irq-mappings for only those gpios which are marked as IRQs, but it requested those gpios too and preconfigured them And that breaks every driver which uses gpios for IRQs. To summarize what happens if a driver uses a gpio as irq: gpio_request() // This works only if the gpio was not requested before gpio_direction_input() gpio_to_irq() // This needs an irq-mapping request_threaded_irq() So I would suggest multiple steps to change that: 1. Create a mapping for every gpio found in DT (or all gpios if no DT is used). I think that is what Linus patch does (sorry, I haven't really followed this thread and didn't look in deep at the patch). 2. Implement gpio_request_for_irq() This would just be a small macro for gpio_request(); gpio_direction_input() 3. Change all drivers which do use gpio_to_irq() to use gpio_request_for_irq() instead of gpio_request() and gpio_direction_input(). This will end up with a big series of more or less trivial patches (I count 677 occurences of gpio_to_irq) and might be splitted. 4. request gpios and set them to input when a gpio is marked as an IRQ in the DT and DT is used. Change gpio_rquest_for_irq() to a NOP if DT is used or leave it as it is for the none-dt case. But I still see a possible problem with requesting a gpio (centralized) if it is marked as IRQ in DT: it does this always. I'm not sure, but there might be cases where this isn't wanted. Just that a GPIO is marked as IRQ in the DT for one driver, doesn't mean that this driver will be really used (and something else might use the GPIO instead). Regards, Alexander Holler -- 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: Odd behavior with dpll4_m4x2_ck on omap3 + DT
On 10/09/13 16:17, Tero Kristo wrote: In theory, DPLLs can also be used in their bypass mode to feed customer nodes clocks. I just think the check in the clkoutx2_recalc is wrong, and should be enhanced to actually check what is the target mode for the clock once it is enabled. Maybe something like this would work properly: diff --git a/arch/arm/mach-omap2/dpll3xxx.c b/arch/arm/mach-omap2/dpll3xxx.c index 3a0296c..ba218fb 100644 --- a/arch/arm/mach-omap2/dpll3xxx.c +++ b/arch/arm/mach-omap2/dpll3xxx.c @@ -658,14 +658,12 @@ unsigned long omap3_clkoutx2_recalc(struct clk_hw *hw, dd = pclk-dpll_data; - WARN_ON(!dd-enable_mask); - - v = __raw_readl(dd-control_reg) dd-enable_mask; - v = __ffs(dd-enable_mask); - if ((v != OMAP3XXX_EN_DPLL_LOCKED) || (dd-flags DPLL_J_TYPE)) + if ((dd-flags DPLL_J_TYPE) || + __clk_get_rate(dd-clk_bypass) == __clk_get_rate(pclk)) rate = parent_rate; else rate = parent_rate * 2; + return rate; } Stefan, are you able to test the above? I'd rather have a proper fix for this, than hack omapdss =). How is the DPLL4's clock rate 43200 anyway in bypass mode. Isn't bypass mode usually plain sys-clock, or such? This again reflects the rate that the clock has once it is enabled, the clkoutx2 does not. Ok. Then it does sound like the clkoutx2 is calculated wrong, as you say. Tomi signature.asc Description: OpenPGP digital signature
Re: [PATCH] RFC: interrupt consistency check for OF GPIO IRQs
And another small update. ;) Am 11.09.2013 09:16, schrieb Alexander Holler: To summarize what happens if a driver uses a gpio as irq: gpio_request() // This works only if the gpio was not requested before gpio_direction_input() gpio_to_irq() // This needs an irq-mapping request_threaded_irq() So I would suggest multiple steps to change that: 1. Create a mapping for every gpio found in DT (or all gpios if no DT is used). I think that is what Linus patch does (sorry, I haven't really followed this thread and didn't look in deep at the patch). 2. Implement gpio_request_for_irq() This would just be a small macro for gpio_request(); gpio_direction_input() I would add gpio_to_irq() to that macro, so that it returns the irq number or zero if something failed. Regards, Alexander Holler -- 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] RFC: interrupt consistency check for OF GPIO IRQs
Am 11.09.2013 09:30, schrieb Alexander Holler: And another small update. ;) Am 11.09.2013 09:16, schrieb Alexander Holler: To summarize what happens if a driver uses a gpio as irq: gpio_request() // This works only if the gpio was not requested before gpio_direction_input() gpio_to_irq() // This needs an irq-mapping request_threaded_irq() So I would suggest multiple steps to change that: 1. Create a mapping for every gpio found in DT (or all gpios if no DT is used). I think that is what Linus patch does (sorry, I haven't really followed this thread and didn't look in deep at the patch). 2. Implement gpio_request_for_irq() This would just be a small macro for gpio_request(); gpio_direction_input() I would add gpio_to_irq() to that macro, so that it returns the irq number or zero if something failed. Doing that, it would be easy to mark gpio_to_irq() as deprecated and driver authors would change automatically to use gpio_request_for_irq() (or gpio_request_as_irq() which might more correct english). So I would suggest just the two steps above, the first with the mapping and the second which introduces gpio_request_as_irq() and marks gpio_to_irq() as deprecated. Everthing else I would leave out for the future (so I wouldn't request gpios centrally). Regards, Alexander Holler -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] am335x-boneblack: add eMMC DT entry
On Wednesday 11 September 2013 11:42 AM, Koen Kooi wrote:
Signed-off-by: Koen Kooi k...@dominion.thruhere.net
---
arch/arm/boot/dts/am335x-bone-common.dtsi | 7 +++
arch/arm/boot/dts/am335x-boneblack.dts| 15 +++
2 files changed, 22 insertions(+)
diff --git a/arch/arm/boot/dts/am335x-bone-common.dtsi
b/arch/arm/boot/dts/am335x-bone-common.dtsi
index ced256c..9c61fef 100644
--- a/arch/arm/boot/dts/am335x-bone-common.dtsi
+++ b/arch/arm/boot/dts/am335x-bone-common.dtsi
@@ -241,6 +241,13 @@
regulator-always-on;
};
};
+
+ vmmcsd_fixed: fixedregulator@0 {
+ compatible = regulator-fixed;
+ regulator-name = vmmcsd_fixed;
+ regulator-min-microvolt = 330;
+ regulator-max-microvolt = 330;
+ };
};
cpsw_emac0 {
diff --git a/arch/arm/boot/dts/am335x-boneblack.dts
b/arch/arm/boot/dts/am335x-boneblack.dts
index 197cadf..c09947a 100644
--- a/arch/arm/boot/dts/am335x-boneblack.dts
+++ b/arch/arm/boot/dts/am335x-boneblack.dts
@@ -15,3 +15,18 @@
regulator-max-microvolt = 180;
regulator-always-on;
};
+
+mmc1 {
+ vmmc-supply = vmmcsd_fixed;
+};
+
+mmc2 {
+ vmmc-supply = vmmcsd_fixed;
+ pinctrl-names = default;
+ pinctrl-0 = emmc_pins;
+ bus-width = 8;
+ ti,non-removable;
There is a standard binding available for this: non-removable
There should not be a need to introduce a TI specific binding for this
(I know this is not your fault).
I will send a patch to change the existing .dts files and update the
driver - can you drop this line for now so we don't introduce more
incompatibilities? I can send a patch adding non-removable to this
node as part of my clean-up series.
Thanks,
Sekhar
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] am335x-boneblack: add eMMC DT entry
Op 11 sep. 2013, om 12:06 heeft Sekhar Nori nsek...@ti.com het volgende
geschreven:
On Wednesday 11 September 2013 11:42 AM, Koen Kooi wrote:
Signed-off-by: Koen Kooi k...@dominion.thruhere.net
---
arch/arm/boot/dts/am335x-bone-common.dtsi | 7 +++
arch/arm/boot/dts/am335x-boneblack.dts| 15 +++
2 files changed, 22 insertions(+)
diff --git a/arch/arm/boot/dts/am335x-bone-common.dtsi
b/arch/arm/boot/dts/am335x-bone-common.dtsi
index ced256c..9c61fef 100644
--- a/arch/arm/boot/dts/am335x-bone-common.dtsi
+++ b/arch/arm/boot/dts/am335x-bone-common.dtsi
@@ -241,6 +241,13 @@
regulator-always-on;
};
};
+
+vmmcsd_fixed: fixedregulator@0 {
+compatible = regulator-fixed;
+regulator-name = vmmcsd_fixed;
+regulator-min-microvolt = 330;
+regulator-max-microvolt = 330;
+};
};
cpsw_emac0 {
diff --git a/arch/arm/boot/dts/am335x-boneblack.dts
b/arch/arm/boot/dts/am335x-boneblack.dts
index 197cadf..c09947a 100644
--- a/arch/arm/boot/dts/am335x-boneblack.dts
+++ b/arch/arm/boot/dts/am335x-boneblack.dts
@@ -15,3 +15,18 @@
regulator-max-microvolt = 180;
regulator-always-on;
};
+
+mmc1 {
+vmmc-supply = vmmcsd_fixed;
+};
+
+mmc2 {
+vmmc-supply = vmmcsd_fixed;
+pinctrl-names = default;
+pinctrl-0 = emmc_pins;
+bus-width = 8;
+ti,non-removable;
There is a standard binding available for this: non-removable
There should not be a need to introduce a TI specific binding for this
(I know this is not your fault).
I will send a patch to change the existing .dts files and update the
driver - can you drop this line for now so we don't introduce more
incompatibilities? I can send a patch adding non-removable to this
node as part of my clean-up series.
Sure, I'll send a v2 shortly. Thank you very much for killing another ti,
prefixed node!
regards,
Koen
--
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
[PATCHv2 0/2] ARM: dts: Beaglebone MMC fixes
Here are two patches to fix MMC on beaglebone, one fixes card detect on BBW, the other adds the eMMC entry for BBB and its fixed regulator. This series depends on: http://comments.gmane.org/gmane.linux.kernel.stable/63648 https://lkml.org/lkml/2013/9/10/454 http://comments.gmane.org/gmane.linux.kernel.mmc/22381 Or as git-cherry would put it: [koen@rrMBP patches]$ git cherry -v + c8436cd15251ea42c7f2decd1cc972de536a7573 ARM: OMAP2+: am335x-bone*: add DT for BeagleBone Black + 34596cb58346612101b168b34b7ee75b8b0e9500 ARM: EDMA: Fix clearing of unused list for DT DMA resources + af7f273dfbeeec7669c01a81b8c02f4af2fcf1b9 ARM: dts: add AM33XX EDMA support + a85f9acc8f48a7cb1a07fb93fcba7bd43aa9c853 ARM: dts: add AM33XX SPI DMA support + 72a42fa0b42051251c554d9f75f42b3e9bc6ec8a ARM: dts: add AM33XX MMC support and documentation + 93205a3735613d674b209c3c75ad3e0a7d6be136 arm: bone: dts: add CD for mmc1 + b1bd98705a2da3d02f197d29ca040008ad8c662c am335x-boneblack: add eMMC DT entry Also available as a git branch at https://github.com/koenkooi/linux/commits/mainline Changes since v1: Removed ti,non-removable entry from eMMC node, see http://marc.info/?l=linux-arm-kernelm=137889435710552w=2 --- Alexander Holler (1): arm: bone: dts: add CD for mmc1 Koen Kooi (1): am335x-boneblack: add eMMC DT entry arch/arm/boot/dts/am335x-bone-common.dtsi | 21 + arch/arm/boot/dts/am335x-bone.dts | 4 arch/arm/boot/dts/am335x-boneblack.dts| 15 +++ 3 files changed, 36 insertions(+), 4 deletions(-) -- 1.8.2.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
[PATCHv2 1/2] arm: bone: dts: add CD for mmc1
From: Alexander Holler hol...@ahsoftware.de
This enables the use of MMC cards even when no card was inserted at boot.
Signed-off-by: Alexander Holler hol...@ahsoftware.de
Signed-off-by: Koen Kooi k...@dominion.thruhere.net
---
Changes since v1:
None, simple repost
arch/arm/boot/dts/am335x-bone-common.dtsi | 14 ++
arch/arm/boot/dts/am335x-bone.dts | 4
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/arch/arm/boot/dts/am335x-bone-common.dtsi
b/arch/arm/boot/dts/am335x-bone-common.dtsi
index 2f66ded..ced256c 100644
--- a/arch/arm/boot/dts/am335x-bone-common.dtsi
+++ b/arch/arm/boot/dts/am335x-bone-common.dtsi
@@ -107,6 +107,11 @@
0x14c (PIN_INPUT_PULLDOWN | MUX_MODE7)
;
};
+ mmc1_pins: pinmux_mmc1_pins {
+ pinctrl-single,pins =
+ 0x160 (PIN_INPUT | MUX_MODE7) /* GPIO0_6 */
+ ;
+ };
};
ocp {
@@ -260,3 +265,12 @@
pinctrl-0 = davinci_mdio_default;
pinctrl-1 = davinci_mdio_sleep;
};
+
+mmc1 {
+ status = okay;
+ vmmc-supply = ldo3_reg;
+ pinctrl-names = default;
+ pinctrl-0 = mmc1_pins;
+ cd-gpios = gpio0 6 GPIO_ACTIVE_HIGH;
+ cd-inverted;
+};
diff --git a/arch/arm/boot/dts/am335x-bone.dts
b/arch/arm/boot/dts/am335x-bone.dts
index d5f43fe..d34469c 100644
--- a/arch/arm/boot/dts/am335x-bone.dts
+++ b/arch/arm/boot/dts/am335x-bone.dts
@@ -16,7 +16,3 @@
regulator-always-on;
};
-mmc1 {
- status = okay;
- vmmc-supply = ldo3_reg;
-};
--
1.8.2.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
[PATCHv2 2/2] am335x-boneblack: add eMMC DT entry
Signed-off-by: Koen Kooi k...@dominion.thruhere.net
---
Changes since v1:
dropped the ti,non-removable entry per Sehkars request
arch/arm/boot/dts/am335x-bone-common.dtsi | 7 +++
arch/arm/boot/dts/am335x-boneblack.dts| 14 ++
2 files changed, 21 insertions(+)
diff --git a/arch/arm/boot/dts/am335x-bone-common.dtsi
b/arch/arm/boot/dts/am335x-bone-common.dtsi
index ced256c..9c61fef 100644
--- a/arch/arm/boot/dts/am335x-bone-common.dtsi
+++ b/arch/arm/boot/dts/am335x-bone-common.dtsi
@@ -241,6 +241,13 @@
regulator-always-on;
};
};
+
+ vmmcsd_fixed: fixedregulator@0 {
+ compatible = regulator-fixed;
+ regulator-name = vmmcsd_fixed;
+ regulator-min-microvolt = 330;
+ regulator-max-microvolt = 330;
+ };
};
cpsw_emac0 {
diff --git a/arch/arm/boot/dts/am335x-boneblack.dts
b/arch/arm/boot/dts/am335x-boneblack.dts
index 197cadf..f4703cf 100644
--- a/arch/arm/boot/dts/am335x-boneblack.dts
+++ b/arch/arm/boot/dts/am335x-boneblack.dts
@@ -15,3 +15,17 @@
regulator-max-microvolt = 180;
regulator-always-on;
};
+
+mmc1 {
+ vmmc-supply = vmmcsd_fixed;
+};
+
+mmc2 {
+ vmmc-supply = vmmcsd_fixed;
+ pinctrl-names = default;
+ pinctrl-0 = emmc_pins;
+ bus-width = 8;
+ status = okay;
+ ti,vcc-aux-disable-is-sleep;
+};
+
--
1.8.2.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: [PATCH] RFC: interrupt consistency check for OF GPIO IRQs
Am 22.08.2013 00:02, schrieb Linus Walleij: On Tue, Aug 20, 2013 at 12:04 AM, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: On Wednesday 31 July 2013 01:44:53 Linus Walleij wrote: I don't see how sharing works here, or how another user, i.e. another one than the user wanting to recieve the IRQ, can validly request such a line? What would the usecase for that valid request be? When the GPIO is wired to a status signal (such as an MMC card detect signal) the driver might want to read the state of the signal independently of the interrupt handler. That is true. But for such a complex usecase I think it's reasonable that we only specify the GPIO in the device tree, and the driver utilizing the IRQ need to take that and perform gpio_to_irq() on it, and then it still works to use it both ways. Hmm, the problem is that DT is seen as fixed. So if someone marks a GPIO as an IRQ, it can never be used otherwise. So if you really go this way, you should make this pretty clear in the documentation. Looking from the other side, why do you want to mark GPIOs as IRQs in the DT at all? And how will this be done? I found the way it was done in the reverted patch very confusing because it needed an IRQ number. That IRQ number depends on the mapping and isn't hw specific (and currently just human doable because of the simple mapping). Regards, Alexander Holler -- 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] RFC: interrupt consistency check for OF GPIO IRQs
On 09/11/2013 05:30 PM, Alexander Holler wrote:
Am 22.08.2013 00:02, schrieb Linus Walleij:
On Tue, Aug 20, 2013 at 12:04 AM, Laurent Pinchart
laurent.pinch...@ideasonboard.com wrote:
On Wednesday 31 July 2013 01:44:53 Linus Walleij wrote:
I don't see how sharing works here, or how another user, i.e. another one
than the user wanting to recieve the IRQ, can validly request such a line?
What would the usecase for that valid request be?
When the GPIO is wired to a status signal (such as an MMC card detect
signal)
the driver might want to read the state of the signal independently of the
interrupt handler.
That is true. But for such a complex usecase I think it's reasonable that
we only specify the GPIO in the device tree, and the driver utilizing the
IRQ need to take that and perform gpio_to_irq() on it, and then it still
works to use it both ways.
Hmm, the problem is that DT is seen as fixed. So if someone marks a GPIO
as an IRQ, it can never be used otherwise. So if you really go this way,
you should make this pretty clear in the documentation.
DT is fixed because that describes the hardware which is fixed of course. So if
a chip IRQ line is connected to a GPIO pin in a controller that should be
described in the DT and that pin can't be used for anything else.
Looking from the other side, why do you want to mark GPIOs as IRQs in
the DT at all?
Because from the component point-of-view that is wired to the SoC, that GPIO is
an IRQ line and so it has to be described.
And how will this be done? I found the way it was done in
the reverted patch very confusing because it needed an IRQ number. That
IRQ number depends on the mapping and isn't hw specific (and currently
just human doable because of the simple mapping).
That's is not true. You don't define an IRQ number what you define is a GPIO
number that is mapped as IRQ. The GPIO number does not depend on the mapping and
it only depends on the GPIO controller.
This has absolutely nothing to do with the reverted patches and is described in
Documentation/devicetree/bindings/interrupt-controller/interrupts.txt.
The only difference is that the reverted patches did actually take an action
when a GPIO pin was mapped as an IRQ (requesting the GPIO and as input).
So for example in an OMAP board DT you can define something like this:
ethernet@5,0 {
compatible = smsc,lan9221, smsc,lan9115;
interrupt-parent = gpio6;
interrupts = 16 8;
};
Since each OMAP GPIO bank has 32 GPIO pins, then what you are defining is that
the GPIO 176 (5 * 32 + 16) will be mapped as the IRQ line for the ethernet
controller.
I explained the exact use case I'm trying to solve in the thread Re: [PATCH v3]
gpio: interrupt consistency check for OF GPIO IRQs [1] if you need more
context. I'm sure others cc'ed in this thread have different (but similar)
requirements.
Regards,
Alexander Holler
Thanks a lot and best regards,
Javier
[1]: http://www.kernelhub.org/?p=2msg=326503
--
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] RFC: interrupt consistency check for OF GPIO IRQs
Am 11.09.2013 18:14, schrieb Javier Martinez Canillas:
On 09/11/2013 05:30 PM, Alexander Holler wrote:
Am 22.08.2013 00:02, schrieb Linus Walleij:
On Tue, Aug 20, 2013 at 12:04 AM, Laurent Pinchart
laurent.pinch...@ideasonboard.com wrote:
On Wednesday 31 July 2013 01:44:53 Linus Walleij wrote:
I don't see how sharing works here, or how another user, i.e. another one
than the user wanting to recieve the IRQ, can validly request such a line?
What would the usecase for that valid request be?
When the GPIO is wired to a status signal (such as an MMC card detect signal)
the driver might want to read the state of the signal independently of the
interrupt handler.
That is true. But for such a complex usecase I think it's reasonable that
we only specify the GPIO in the device tree, and the driver utilizing the
IRQ need to take that and perform gpio_to_irq() on it, and then it still
works to use it both ways.
Hmm, the problem is that DT is seen as fixed. So if someone marks a GPIO
as an IRQ, it can never be used otherwise. So if you really go this way,
you should make this pretty clear in the documentation.
DT is fixed because that describes the hardware which is fixed of course. So if
a chip IRQ line is connected to a GPIO pin in a controller that should be
described in the DT and that pin can't be used for anything else.
If you request an gpio for every gpio which defined as IRQ centrally,
drivers can't just use that gpio as gpio afterwards. And there might be
one driver which uses the irq but another one which wants the gpio. But
if you define a gpio as irq in dt, you take away that choice.
Looking from the other side, why do you want to mark GPIOs as IRQs in
the DT at all?
Because from the component point-of-view that is wired to the SoC, that GPIO is
an IRQ line and so it has to be described.
And how will this be done? I found the way it was done in
the reverted patch very confusing because it needed an IRQ number. That
IRQ number depends on the mapping and isn't hw specific (and currently
just human doable because of the simple mapping).
That's is not true. You don't define an IRQ number what you define is a GPIO
number that is mapped as IRQ. The GPIO number does not depend on the mapping and
it only depends on the GPIO controller.
This has absolutely nothing to do with the reverted patches and is described in
Documentation/devicetree/bindings/interrupt-controller/interrupts.txt.
The only difference is that the reverted patches did actually take an action
when a GPIO pin was mapped as an IRQ (requesting the GPIO and as input).
So for example in an OMAP board DT you can define something like this:
ethernet@5,0 {
compatible = smsc,lan9221, smsc,lan9115;
interrupt-parent = gpio6;
interrupts = 16 8;
};
Since each OMAP GPIO bank has 32 GPIO pins, then what you are defining is that
the GPIO 176 (5 * 32 + 16) will be mapped as the IRQ line for the ethernet
controller.
I explained the exact use case I'm trying to solve in the thread Re: [PATCH v3]
gpio: interrupt consistency check for OF GPIO IRQs [1] if you need more
context. I'm sure others cc'ed in this thread have different (but similar)
requirements.
So I would extend my previous proposal
(http://www.mail-archive.com/linux-omap@vger.kernel.org/msg95202.html)
for a gpio_request_as_irq() such, that I would change that to
request_irq_new(number, irq_controller) where the irq_controller would
be the gpio-controller in case of omap (while marking gpio_to_irq() as
deprecated).
So request_irq_new() would than do the following on omap if the
irq-controller is a gpio-controller:
gpio_request() // This works only if the gpio was not requested before
gpio_direction_input()
(build irq-mapping)
gpio_to_irq() // This needs an irq-mapping
return request_threaded_irq()
and all drivers could replace the above sequence just with
request_irq_new(number_from_dt, irq-controller_from_dt).
How's that?
Regards,
Alexander Holler
--
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: commit 5fe212364 causes division by zero with large bauds
Hi,
On Wed, Sep 11, 2013 at 09:22:26AM +0300, Alexey Pelykh wrote:
Hi Felipe,
Thanks for finding this issue. Indeed, there is a bug on 3M+ baud
rates. First patch is close to a complete fix, but still contains
div-by-zero issue. Here is my version:
diff --git a/drivers/tty/serial/omap-serial.c
b/drivers/tty/serial/omap-serial.c
index 816d1a2..808a880 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -240,8 +240,8 @@ serial_omap_baud_is_mode16(struct uart_port *port,
unsigned int baud)
{
unsigned int n13 = port-uartclk / (13 * baud);
unsigned int n16 = port-uartclk / (16 * baud);
- int baudAbsDiff13 = baud - (port-uartclk / (13 * n13));
- int baudAbsDiff16 = baud - (port-uartclk / (16 * n16));
+ int baudAbsDiff13 = n13 ? (baud - (port-uartclk / (13 * n13))) :
INT_MAX;
+ int baudAbsDiff16 = n16 ? (baud - (port-uartclk / (16 * n16))) :
INT_MAX;
IOW:
int baudAbsDiff13 = 0;
if (n13)
baudAbsDiff13 = (baud - (port-uartclk / (13 * n13)));
which is exactly what my patch did. I fail to see where division by zero
would be coming from.
if(baudAbsDiff13 0)
baudAbsDiff13 = -baudAbsDiff13;
if(baudAbsDiff16 0)
With 48MHz UART clock, it will give
300: divisor = 12307 (13), real rate 300 (0.00%)
600: divisor = 6153 (13), real rate 600 (0.00%)
1200: divisor = 3076 (13), real rate 1200 (0.00%)
2400: divisor = 1538 (13), real rate 2400 (0.00%)
TRM has these all set with oversampling of 16. In fact only 460800,
921600, 1843200 and 3686400 should be using oversampling of 13.
--
balbi
signature.asc
Description: Digital signature
Re: commit 5fe212364 causes division by zero with large bauds
On Wed, Sep 11, 2013 at 9:38 PM, Felipe Balbi ba...@ti.com wrote:
Hi,
On Wed, Sep 11, 2013 at 09:22:26AM +0300, Alexey Pelykh wrote:
Hi Felipe,
Thanks for finding this issue. Indeed, there is a bug on 3M+ baud
rates. First patch is close to a complete fix, but still contains
div-by-zero issue. Here is my version:
diff --git a/drivers/tty/serial/omap-serial.c
b/drivers/tty/serial/omap-serial.c
index 816d1a2..808a880 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -240,8 +240,8 @@ serial_omap_baud_is_mode16(struct uart_port *port,
unsigned int baud)
{
unsigned int n13 = port-uartclk / (13 * baud);
unsigned int n16 = port-uartclk / (16 * baud);
- int baudAbsDiff13 = baud - (port-uartclk / (13 * n13));
- int baudAbsDiff16 = baud - (port-uartclk / (16 * n16));
+ int baudAbsDiff13 = n13 ? (baud - (port-uartclk / (13 * n13))) :
INT_MAX;
+ int baudAbsDiff16 = n16 ? (baud - (port-uartclk / (16 * n16))) :
INT_MAX;
IOW:
int baudAbsDiff13 = 0;
if (n13)
baudAbsDiff13 = (baud - (port-uartclk / (13 * n13)));
Not quite same code, INT_MAX instead of 0. With 0 a div-by-zero
exception will still occur on 3686400.
which is exactly what my patch did. I fail to see where division by zero
would be coming from.
if(baudAbsDiff13 0)
baudAbsDiff13 = -baudAbsDiff13;
if(baudAbsDiff16 0)
With 48MHz UART clock, it will give
300: divisor = 12307 (13), real rate 300 (0.00%)
600: divisor = 6153 (13), real rate 600 (0.00%)
1200: divisor = 3076 (13), real rate 1200 (0.00%)
2400: divisor = 1538 (13), real rate 2400 (0.00%)
TRM has these all set with oversampling of 16. In fact only 460800,
921600, 1843200 and 3686400 should be using oversampling of 13.
That's true, but TRM anyways does not contain all possible baud rates
(1M e.g.). IMO, as long as error rate is the same as in TRM,
it makes no difference what combination of (mode, divisor) to use.
--
balbi
A complex solution may be implemented: use LUT for baud rates that TRM
defines explicitly, and use calculation if lookup failed.
Thanks,
Alexey
--
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: commit 5fe212364 causes division by zero with large bauds
On Wed, Sep 11, 2013 at 09:48:13PM +0300, Alexey Pelykh wrote:
On Wed, Sep 11, 2013 at 9:38 PM, Felipe Balbi ba...@ti.com wrote:
Hi,
On Wed, Sep 11, 2013 at 09:22:26AM +0300, Alexey Pelykh wrote:
Hi Felipe,
Thanks for finding this issue. Indeed, there is a bug on 3M+ baud
rates. First patch is close to a complete fix, but still contains
div-by-zero issue. Here is my version:
diff --git a/drivers/tty/serial/omap-serial.c
b/drivers/tty/serial/omap-serial.c
index 816d1a2..808a880 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -240,8 +240,8 @@ serial_omap_baud_is_mode16(struct uart_port *port,
unsigned int baud)
{
unsigned int n13 = port-uartclk / (13 * baud);
unsigned int n16 = port-uartclk / (16 * baud);
- int baudAbsDiff13 = baud - (port-uartclk / (13 * n13));
- int baudAbsDiff16 = baud - (port-uartclk / (16 * n16));
+ int baudAbsDiff13 = n13 ? (baud - (port-uartclk / (13 * n13))) :
INT_MAX;
+ int baudAbsDiff16 = n16 ? (baud - (port-uartclk / (16 * n16))) :
INT_MAX;
IOW:
int baudAbsDiff13 = 0;
if (n13)
baudAbsDiff13 = (baud - (port-uartclk / (13 * n13)));
Not quite same code, INT_MAX instead of 0. With 0 a div-by-zero
exception will still occur on 3686400.
why, there's no division after that point, right ? Besides,
serial_omap_baud_is_mode16() is supposed to return a boolean value.
Setting baudAbsDiff1[36] to 0 will cause no problems, you're only using
that value with a boolean expression, no divisions whatsoever. Division
is done after using serial_omap_baud_is_mode16() to initialize divisor
to 13 or 16 (which is a misnamer, since that's the oversampling
parameter).
which is exactly what my patch did. I fail to see where division by zero
would be coming from.
if(baudAbsDiff13 0)
baudAbsDiff13 = -baudAbsDiff13;
if(baudAbsDiff16 0)
With 48MHz UART clock, it will give
300: divisor = 12307 (13), real rate 300 (0.00%)
600: divisor = 6153 (13), real rate 600 (0.00%)
1200: divisor = 3076 (13), real rate 1200 (0.00%)
2400: divisor = 1538 (13), real rate 2400 (0.00%)
TRM has these all set with oversampling of 16. In fact only 460800,
921600, 1843200 and 3686400 should be using oversampling of 13.
That's true, but TRM anyways does not contain all possible baud rates
(1M e.g.). IMO, as long as error rate is the same as in TRM,
it makes no difference what combination of (mode, divisor) to use.
--
balbi
A complex solution may be implemented: use LUT for baud rates that TRM
defines explicitly, and use calculation if lookup failed.
why would you try calculating anything if there is nothing in the table
which can support it ? Whatever is in the lookup table, are the only
baud rates the SoC supports, right ?
cheers
--
balbi
signature.asc
Description: Digital signature
Re: commit 5fe212364 causes division by zero with large bauds
On Wed, Sep 11, 2013 at 10:00 PM, Felipe Balbi ba...@ti.com wrote:
On Wed, Sep 11, 2013 at 09:48:13PM +0300, Alexey Pelykh wrote:
On Wed, Sep 11, 2013 at 9:38 PM, Felipe Balbi ba...@ti.com wrote:
Hi,
On Wed, Sep 11, 2013 at 09:22:26AM +0300, Alexey Pelykh wrote:
Hi Felipe,
Thanks for finding this issue. Indeed, there is a bug on 3M+ baud
rates. First patch is close to a complete fix, but still contains
div-by-zero issue. Here is my version:
diff --git a/drivers/tty/serial/omap-serial.c
b/drivers/tty/serial/omap-serial.c
index 816d1a2..808a880 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -240,8 +240,8 @@ serial_omap_baud_is_mode16(struct uart_port *port,
unsigned int baud)
{
unsigned int n13 = port-uartclk / (13 * baud);
unsigned int n16 = port-uartclk / (16 * baud);
- int baudAbsDiff13 = baud - (port-uartclk / (13 * n13));
- int baudAbsDiff16 = baud - (port-uartclk / (16 * n16));
+ int baudAbsDiff13 = n13 ? (baud - (port-uartclk / (13 * n13))) :
INT_MAX;
+ int baudAbsDiff16 = n16 ? (baud - (port-uartclk / (16 * n16))) :
INT_MAX;
IOW:
int baudAbsDiff13 = 0;
if (n13)
baudAbsDiff13 = (baud - (port-uartclk / (13 * n13)));
Not quite same code, INT_MAX instead of 0. With 0 a div-by-zero
exception will still occur on 3686400.
why, there's no division after that point, right ? Besides,
serial_omap_baud_is_mode16() is supposed to return a boolean value.
Setting baudAbsDiff1[36] to 0 will cause no problems, you're only using
that value with a boolean expression, no divisions whatsoever. Division
is done after using serial_omap_baud_is_mode16() to initialize divisor
to 13 or 16 (which is a misnamer, since that's the oversampling
parameter).
Yes, variables are a bit misnamed, that should be fixed someday.
Regarding 0 vs INT_MAX, in case of 0 values will be
300: divisor = 12307 (13)
600: divisor = 6153 (13)
1200: divisor = 3076 (13)
2400: divisor = 1538 (13)
4800: divisor = 625 (16)
9600: divisor = 384 (13)
14400: divisor = 256 (13)
19200: divisor = 192 (13)
28800: divisor = 128 (13)
38400: divisor = 96 (13)
57600: divisor = 64 (13)
115200: divisor = 32 (13)
230400: divisor = 16 (13)
460800: divisor = 8 (13)
921600: divisor = 4 (13)
100: divisor = 3 (16)
1843200: divisor = 2 (13)
300: divisor = 1 (16)
3686400: divisor = 0 (16) error here, should be 1 (13), as it is with INT_MAX
which is exactly what my patch did. I fail to see where division by zero
would be coming from.
if(baudAbsDiff13 0)
baudAbsDiff13 = -baudAbsDiff13;
if(baudAbsDiff16 0)
With 48MHz UART clock, it will give
300: divisor = 12307 (13), real rate 300 (0.00%)
600: divisor = 6153 (13), real rate 600 (0.00%)
1200: divisor = 3076 (13), real rate 1200 (0.00%)
2400: divisor = 1538 (13), real rate 2400 (0.00%)
TRM has these all set with oversampling of 16. In fact only 460800,
921600, 1843200 and 3686400 should be using oversampling of 13.
That's true, but TRM anyways does not contain all possible baud rates
(1M e.g.). IMO, as long as error rate is the same as in TRM,
it makes no difference what combination of (mode, divisor) to use.
--
balbi
A complex solution may be implemented: use LUT for baud rates that TRM
defines explicitly, and use calculation if lookup failed.
why would you try calculating anything if there is nothing in the table
which can support it ? Whatever is in the lookup table, are the only
baud rates the SoC supports, right ?
Actually, I haven't found any statement in TRM, which would mention
that listed baudrates in referenced table are the only supported baud
rates,
and all others are illegal.
At least 1M which I use extensively works perfectly, and I can not
figure out any idea why it would not do so.
General idea behind original commit is to provide actual baudrate
that is as close as possible to requested one,
while keeping values in allowed range of divisor and mode.
cheers
--
balbi
Best regards,
Alexey
--
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: commit 5fe212364 causes division by zero with large bauds
Hi,
On Wed, Sep 11, 2013 at 10:19:47PM +0300, Alexey Pelykh wrote:
On Wed, Sep 11, 2013 at 10:00 PM, Felipe Balbi ba...@ti.com wrote:
On Wed, Sep 11, 2013 at 09:48:13PM +0300, Alexey Pelykh wrote:
On Wed, Sep 11, 2013 at 9:38 PM, Felipe Balbi ba...@ti.com wrote:
Hi,
On Wed, Sep 11, 2013 at 09:22:26AM +0300, Alexey Pelykh wrote:
Hi Felipe,
Thanks for finding this issue. Indeed, there is a bug on 3M+ baud
rates. First patch is close to a complete fix, but still contains
div-by-zero issue. Here is my version:
diff --git a/drivers/tty/serial/omap-serial.c
b/drivers/tty/serial/omap-serial.c
index 816d1a2..808a880 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -240,8 +240,8 @@ serial_omap_baud_is_mode16(struct uart_port *port,
unsigned int baud)
{
unsigned int n13 = port-uartclk / (13 * baud);
unsigned int n16 = port-uartclk / (16 * baud);
- int baudAbsDiff13 = baud - (port-uartclk / (13 * n13));
- int baudAbsDiff16 = baud - (port-uartclk / (16 * n16));
+ int baudAbsDiff13 = n13 ? (baud - (port-uartclk / (13 * n13)))
: INT_MAX;
+ int baudAbsDiff16 = n16 ? (baud - (port-uartclk / (16 * n16)))
: INT_MAX;
IOW:
int baudAbsDiff13 = 0;
if (n13)
baudAbsDiff13 = (baud - (port-uartclk / (13 * n13)));
Not quite same code, INT_MAX instead of 0. With 0 a div-by-zero
exception will still occur on 3686400.
why, there's no division after that point, right ? Besides,
serial_omap_baud_is_mode16() is supposed to return a boolean value.
Setting baudAbsDiff1[36] to 0 will cause no problems, you're only using
that value with a boolean expression, no divisions whatsoever. Division
is done after using serial_omap_baud_is_mode16() to initialize divisor
to 13 or 16 (which is a misnamer, since that's the oversampling
parameter).
Yes, variables are a bit misnamed, that should be fixed someday.
Regarding 0 vs INT_MAX, in case of 0 values will be
300: divisor = 12307 (13)
600: divisor = 6153 (13)
1200: divisor = 3076 (13)
2400: divisor = 1538 (13)
4800: divisor = 625 (16)
9600: divisor = 384 (13)
14400: divisor = 256 (13)
19200: divisor = 192 (13)
28800: divisor = 128 (13)
38400: divisor = 96 (13)
57600: divisor = 64 (13)
115200: divisor = 32 (13)
230400: divisor = 16 (13)
460800: divisor = 8 (13)
921600: divisor = 4 (13)
100: divisor = 3 (16)
1843200: divisor = 2 (13)
300: divisor = 1 (16)
3686400: divisor = 0 (16) error here, should be 1 (13), as it is with
INT_MAX
I get it now... your boolean check wants to use the closer baud to
requested baud, so it's mode16 if the delta between baudAbsDiff16 and
requested rate is less than delta between baudAbsDiff13 and requested
baud.
which is exactly what my patch did. I fail to see where division by zero
would be coming from.
if(baudAbsDiff13 0)
baudAbsDiff13 = -baudAbsDiff13;
if(baudAbsDiff16 0)
With 48MHz UART clock, it will give
300: divisor = 12307 (13), real rate 300 (0.00%)
600: divisor = 6153 (13), real rate 600 (0.00%)
1200: divisor = 3076 (13), real rate 1200 (0.00%)
2400: divisor = 1538 (13), real rate 2400 (0.00%)
TRM has these all set with oversampling of 16. In fact only 460800,
921600, 1843200 and 3686400 should be using oversampling of 13.
That's true, but TRM anyways does not contain all possible baud rates
(1M e.g.). IMO, as long as error rate is the same as in TRM,
it makes no difference what combination of (mode, divisor) to use.
--
balbi
A complex solution may be implemented: use LUT for baud rates that TRM
defines explicitly, and use calculation if lookup failed.
why would you try calculating anything if there is nothing in the table
which can support it ? Whatever is in the lookup table, are the only
baud rates the SoC supports, right ?
Actually, I haven't found any statement in TRM, which would mention
that listed baudrates in referenced table are the only supported baud
rates,
and all others are illegal.
The UART clocks are connected to produce a baud rate of up to 3.6 Mbps.
Table 24-122 lists the *supported* baud rates, requested divisor, and
corresponding error versus the standard baud rate.
At least 1M which I use extensively works perfectly, and I can not
figure out any idea why it would not do so.
it might very well work, but it's not officially *supported* by the IP.
--
balbi
signature.asc
Description: Digital signature
Re: commit 5fe212364 causes division by zero with large bauds
On Wed, Sep 11, 2013 at 11:47 PM, Felipe Balbi ba...@ti.com wrote:
Hi,
On Wed, Sep 11, 2013 at 10:19:47PM +0300, Alexey Pelykh wrote:
On Wed, Sep 11, 2013 at 10:00 PM, Felipe Balbi ba...@ti.com wrote:
On Wed, Sep 11, 2013 at 09:48:13PM +0300, Alexey Pelykh wrote:
On Wed, Sep 11, 2013 at 9:38 PM, Felipe Balbi ba...@ti.com wrote:
Hi,
On Wed, Sep 11, 2013 at 09:22:26AM +0300, Alexey Pelykh wrote:
Hi Felipe,
Thanks for finding this issue. Indeed, there is a bug on 3M+ baud
rates. First patch is close to a complete fix, but still contains
div-by-zero issue. Here is my version:
diff --git a/drivers/tty/serial/omap-serial.c
b/drivers/tty/serial/omap-serial.c
index 816d1a2..808a880 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -240,8 +240,8 @@ serial_omap_baud_is_mode16(struct uart_port *port,
unsigned int baud)
{
unsigned int n13 = port-uartclk / (13 * baud);
unsigned int n16 = port-uartclk / (16 * baud);
- int baudAbsDiff13 = baud - (port-uartclk / (13 * n13));
- int baudAbsDiff16 = baud - (port-uartclk / (16 * n16));
+ int baudAbsDiff13 = n13 ? (baud - (port-uartclk / (13 *
n13))) : INT_MAX;
+ int baudAbsDiff16 = n16 ? (baud - (port-uartclk / (16 *
n16))) : INT_MAX;
IOW:
int baudAbsDiff13 = 0;
if (n13)
baudAbsDiff13 = (baud - (port-uartclk / (13 * n13)));
Not quite same code, INT_MAX instead of 0. With 0 a div-by-zero
exception will still occur on 3686400.
why, there's no division after that point, right ? Besides,
serial_omap_baud_is_mode16() is supposed to return a boolean value.
Setting baudAbsDiff1[36] to 0 will cause no problems, you're only using
that value with a boolean expression, no divisions whatsoever. Division
is done after using serial_omap_baud_is_mode16() to initialize divisor
to 13 or 16 (which is a misnamer, since that's the oversampling
parameter).
Yes, variables are a bit misnamed, that should be fixed someday.
Regarding 0 vs INT_MAX, in case of 0 values will be
300: divisor = 12307 (13)
600: divisor = 6153 (13)
1200: divisor = 3076 (13)
2400: divisor = 1538 (13)
4800: divisor = 625 (16)
9600: divisor = 384 (13)
14400: divisor = 256 (13)
19200: divisor = 192 (13)
28800: divisor = 128 (13)
38400: divisor = 96 (13)
57600: divisor = 64 (13)
115200: divisor = 32 (13)
230400: divisor = 16 (13)
460800: divisor = 8 (13)
921600: divisor = 4 (13)
100: divisor = 3 (16)
1843200: divisor = 2 (13)
300: divisor = 1 (16)
3686400: divisor = 0 (16) error here, should be 1 (13), as it is with
INT_MAX
I get it now... your boolean check wants to use the closer baud to
requested baud, so it's mode16 if the delta between baudAbsDiff16 and
requested rate is less than delta between baudAbsDiff13 and requested
baud.
which is exactly what my patch did. I fail to see where division by zero
would be coming from.
if(baudAbsDiff13 0)
baudAbsDiff13 = -baudAbsDiff13;
if(baudAbsDiff16 0)
With 48MHz UART clock, it will give
300: divisor = 12307 (13), real rate 300 (0.00%)
600: divisor = 6153 (13), real rate 600 (0.00%)
1200: divisor = 3076 (13), real rate 1200 (0.00%)
2400: divisor = 1538 (13), real rate 2400 (0.00%)
TRM has these all set with oversampling of 16. In fact only 460800,
921600, 1843200 and 3686400 should be using oversampling of 13.
That's true, but TRM anyways does not contain all possible baud rates
(1M e.g.). IMO, as long as error rate is the same as in TRM,
it makes no difference what combination of (mode, divisor) to use.
--
balbi
A complex solution may be implemented: use LUT for baud rates that TRM
defines explicitly, and use calculation if lookup failed.
why would you try calculating anything if there is nothing in the table
which can support it ? Whatever is in the lookup table, are the only
baud rates the SoC supports, right ?
Actually, I haven't found any statement in TRM, which would mention
that listed baudrates in referenced table are the only supported baud
rates,
and all others are illegal.
The UART clocks are connected to produce a baud rate of up to 3.6 Mbps.
Table 24-122 lists the *supported* baud rates, requested divisor, and
corresponding error versus the standard baud rate.
At least 1M which I use extensively works perfectly, and I can not
figure out any idea why it would not do so.
it might very well work, but it's not officially *supported* by the IP.
That's true, but I don't see any reason why driver should disallow
usage of baud rates that are not supported, but possible by hardware:
The UART clocks are connected to produce a baud rate of up to 3.6M bits/s.
--
balbi
I've changed calculation a bit to give priority to mode16, and now it
gives TRM table as-is + extra baud rates
Re: commit 5fe212364 causes division by zero with large bauds
Actually, here it is, but not formatted properly
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 816d1a2..146e712 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -240,14 +240,14 @@ serial_omap_baud_is_mode16(struct uart_port
*port, unsigned int baud)
{
unsigned int n13 = port-uartclk / (13 * baud);
unsigned int n16 = port-uartclk / (16 * baud);
- int baudAbsDiff13 = baud - (port-uartclk / (13 * n13));
- int baudAbsDiff16 = baud - (port-uartclk / (16 * n16));
+ int baudAbsDiff13 = n13 ? (baud - (port-uartclk / (13 * n13))) : INT_MAX;
+ int baudAbsDiff16 = n16 ? (baud - (port-uartclk / (16 * n16))) : INT_MAX;
if(baudAbsDiff13 0)
baudAbsDiff13 = -baudAbsDiff13;
if(baudAbsDiff16 0)
baudAbsDiff16 = -baudAbsDiff16;
- return (baudAbsDiff13 baudAbsDiff16);
+ return (baudAbsDiff13 = baudAbsDiff16);
}
/*
@@ -258,13 +258,13 @@ serial_omap_baud_is_mode16(struct uart_port
*port, unsigned int baud)
static unsigned int
serial_omap_get_divisor(struct uart_port *port, unsigned int baud)
{
- unsigned int divisor;
+ unsigned int mode;
if (!serial_omap_baud_is_mode16(port, baud))
- divisor = 13;
+ mode = 13;
else
- divisor = 16;
- return port-uartclk/(baud * divisor);
+ mode = 16;
+ return port-uartclk/(mode * baud);
}
static void serial_omap_enable_ms(struct uart_port *port)
On Thu, Sep 12, 2013 at 7:32 AM, Alexey Pelykh alexey.pel...@gmail.com wrote:
On Wed, Sep 11, 2013 at 11:47 PM, Felipe Balbi ba...@ti.com wrote:
Hi,
On Wed, Sep 11, 2013 at 10:19:47PM +0300, Alexey Pelykh wrote:
On Wed, Sep 11, 2013 at 10:00 PM, Felipe Balbi ba...@ti.com wrote:
On Wed, Sep 11, 2013 at 09:48:13PM +0300, Alexey Pelykh wrote:
On Wed, Sep 11, 2013 at 9:38 PM, Felipe Balbi ba...@ti.com wrote:
Hi,
On Wed, Sep 11, 2013 at 09:22:26AM +0300, Alexey Pelykh wrote:
Hi Felipe,
Thanks for finding this issue. Indeed, there is a bug on 3M+ baud
rates. First patch is close to a complete fix, but still contains
div-by-zero issue. Here is my version:
diff --git a/drivers/tty/serial/omap-serial.c
b/drivers/tty/serial/omap-serial.c
index 816d1a2..808a880 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -240,8 +240,8 @@ serial_omap_baud_is_mode16(struct uart_port *port,
unsigned int baud)
{
unsigned int n13 = port-uartclk / (13 * baud);
unsigned int n16 = port-uartclk / (16 * baud);
- int baudAbsDiff13 = baud - (port-uartclk / (13 * n13));
- int baudAbsDiff16 = baud - (port-uartclk / (16 * n16));
+ int baudAbsDiff13 = n13 ? (baud - (port-uartclk / (13 *
n13))) : INT_MAX;
+ int baudAbsDiff16 = n16 ? (baud - (port-uartclk / (16 *
n16))) : INT_MAX;
IOW:
int baudAbsDiff13 = 0;
if (n13)
baudAbsDiff13 = (baud - (port-uartclk / (13 * n13)));
Not quite same code, INT_MAX instead of 0. With 0 a div-by-zero
exception will still occur on 3686400.
why, there's no division after that point, right ? Besides,
serial_omap_baud_is_mode16() is supposed to return a boolean value.
Setting baudAbsDiff1[36] to 0 will cause no problems, you're only using
that value with a boolean expression, no divisions whatsoever. Division
is done after using serial_omap_baud_is_mode16() to initialize divisor
to 13 or 16 (which is a misnamer, since that's the oversampling
parameter).
Yes, variables are a bit misnamed, that should be fixed someday.
Regarding 0 vs INT_MAX, in case of 0 values will be
300: divisor = 12307 (13)
600: divisor = 6153 (13)
1200: divisor = 3076 (13)
2400: divisor = 1538 (13)
4800: divisor = 625 (16)
9600: divisor = 384 (13)
14400: divisor = 256 (13)
19200: divisor = 192 (13)
28800: divisor = 128 (13)
38400: divisor = 96 (13)
57600: divisor = 64 (13)
115200: divisor = 32 (13)
230400: divisor = 16 (13)
460800: divisor = 8 (13)
921600: divisor = 4 (13)
100: divisor = 3 (16)
1843200: divisor = 2 (13)
300: divisor = 1 (16)
3686400: divisor = 0 (16) error here, should be 1 (13), as it is with
INT_MAX
I get it now... your boolean check wants to use the closer baud to
requested baud, so it's mode16 if the delta between baudAbsDiff16 and
requested rate is less than delta between baudAbsDiff13 and requested
baud.
which is exactly what my patch did. I fail to see where division by
zero
would be coming from.
if(baudAbsDiff13 0)
baudAbsDiff13 = -baudAbsDiff13;
if(baudAbsDiff16 0)
With 48MHz UART clock, it will give
300: divisor = 12307 (13), real rate 300 (0.00%)
600: divisor = 6153 (13), real rate 600 (0.00%)
1200: divisor = 3076 (13), real rate 1200 (0.00%)
2400: divisor = 1538 (13), real rate 2400 (0.00%)
TRM has these all set with oversampling of 16. In fact only 460800,
921600, 1843200 and 3686400 should be using
