Re: [PATCH] ARM: davinci: Remove redundant casts
Hello. On 10/21/2014 06:53 PM, Rasmus Villemoes wrote: These casts to char* are unnecessary and slightly confusing, since both operands actually have type const char*. Both operands of what? Typecast only has 1 operand. :-) Signed-off-by: Rasmus Villemoes li...@rasmusvillemoes.dk WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 5/6] ARM: DTS: da850-evm: Add node for tlv320aic3106 codec
Hello.
On 01-08-2014 9:22, Peter Ujfalusi wrote:
The board uses aic3106 for audio.
Signed-off-by: Peter Ujfalusi peter.ujfal...@ti.com
---
arch/arm/boot/dts/da850-evm.dts | 14 ++
1 file changed, 14 insertions(+)
diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
index 09118c72e83f..b9ef2be0b145 100644
--- a/arch/arm/boot/dts/da850-evm.dts
+++ b/arch/arm/boot/dts/da850-evm.dts
@@ -51,6 +51,20 @@
tps: tps@48 {
reg = 0x48;
};
+tlv320aic3106: tlv320aic3106@1b {
[...]
Also, the ePAPR standard [1] says:
The name of a node should be somewhat generic, reflecting the function of the
device and not its precise programming model.
True. This is why the node for the audio support is named as 'sound'. For the
components, like in this case I do not see issue to call the audio codec with
it's name.
I do. We should follow the standard consistently. Why not call the node
sound-codec?
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 5/6] ARM: DTS: da850-evm: Add node for tlv320aic3106 codec
Hello. On 08/01/2014 05:02 PM, Peter Ujfalusi wrote: I do. We should follow the standard consistently. Why not call the node sound-codec? Well, there is _zero_ cases when the audio codec node is named as sound-codec in linux-next but we have wm, tlv, twl, max etc. Which only means people don't read the standard (which is referred to on http://www.devicetree.org/Device_Tree_Usage, that says the same). Yeah, there are few DTS files which have codec as node name. So, no, I'm not going to change the node name from tlv320aic3106. So you prefer following the bad examples to following the standard? Well, the Moor has done his duty, the Moor can go... WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 2/6] ARM: DTS: da850: Add node for edma0
On 07/31/2014 02:18 PM, Peter Ujfalusi wrote:
Add DT node for edma0.
Signed-off-by: Peter Ujfalusi peter.ujfal...@ti.com
---
arch/arm/boot/dts/da850.dtsi | 6 ++
1 file changed, 6 insertions(+)
diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index b695548dbb4e..41ce4e8bf227 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -150,6 +150,12 @@
};
};
+ edma0: edma@01c0 {
+ compatible = ti,edma3;
+ reg = 0x0 0x1;
Why the mismatch between the unit-address part of the node name and the
reg property?
+ interrupts = 11 13 12;
+ #dma-cells = 1;
+ };
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2 1/4] mdio_bus: implement devm_mdiobus_alloc/devm_mdiobus_free
Hello.
On 04/18/2014 09:24 PM, Grygorii Strashko wrote:
Add a resource managed devm_mdiobus_alloc()/devm_mdiobus_free()
to automatically clean up MDIO bus alocations made by MDIO drivers,
thus leading to simplified MDIO drivers code.
Cc: Florian Fainelli f.faine...@gmail.com
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
[...]
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 76f54b3..6412beb 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -69,6 +69,74 @@ struct mii_bus *mdiobus_alloc_size(size_t size)
[...]
+/**
+ * devm_mdiobus_free - Resource-managed mdiobus_free()
+ * @dev: Device this mii_bus belongs to
+ * @bus: the mii_bus associated with the device
+ *
+ * Free mii_bus allocated with devm_mdiobus_alloc().
+ */
+void devm_mdiobus_free(struct device *dev, struct mii_bus *bus)
+{
+ int rc;
+
+ rc = devres_release(dev, _devm_mdiobus_free,
+ devm_mdiobus_match, bus);
Please re-align this line, so that it starts right under 'dev' on the
previous line.
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2 1/4] mdio_bus: implement devm_mdiobus_alloc/devm_mdiobus_free
On 04/18/2014 09:24 PM, Grygorii Strashko wrote:
Add a resource managed devm_mdiobus_alloc()/devm_mdiobus_free()
to automatically clean up MDIO bus alocations made by MDIO drivers,
thus leading to simplified MDIO drivers code.
Cc: Florian Fainelli f.faine...@gmail.com
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
[...]
index 76f54b3..6412beb 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -69,6 +69,74 @@ struct mii_bus *mdiobus_alloc_size(size_t size)
[...]
+/**
+ * devm_mdiobus_alloc - Resource-managed mdiobus_alloc_size()
+ * @dev: Device to allocate mii_bus for
+ * @sizeof_priv: Space to allocate for private structure.
+ *
+ * Managed mdiobus_alloc_size. mii_bus allocated with this function is
+ * automatically freed on driver detach.
+ *
+ * If an mii_bus allocated with this function needs to be freed separately,
+ * devm_mdiobus_free() must be used.
+ *
+ * RETURNS:
+ * Pointer to allocated mii_bus on success, NULL on failure.
+ */
+struct mii_bus *devm_mdiobus_alloc(struct device *dev, int sizeof_priv)
+{
+ struct mii_bus **ptr, *bus;
+
+ ptr = devres_alloc(_devm_mdiobus_free, sizeof(*ptr),
+ GFP_KERNEL);
+ if (!ptr)
+ return NULL;
+
+ /* use raw alloc_dr for kmalloc caller tracing */
+ bus = mdiobus_alloc_size(sizeof_priv);
Since the wrapped function is called mdiobus_alloc_size(), not
mdiobus_alloc(), perhaps it's better to call the wrapper
devm_mdiobus_alloc_size()?
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 1/2] mdio_bus: implement devm_mdiobus_alloc/devm_mdiobus_free
Hello.
On 04-04-2014 17:40, Grygorii Strashko wrote:
Add a resource managed devm_mdiobus_alloc()/devm_mdiobus_free()
to automatically clean up MDIO bus alocations made by MDIO drivers,
thus leading to simplified MDIO drivers code.
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
[...]
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 76f54b3..fdcd6d1 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -69,6 +69,74 @@ struct mii_bus *mdiobus_alloc_size(size_t size)
}
EXPORT_SYMBOL(mdiobus_alloc_size);
+static void _devm_mdiobus_free(struct device *dev, void *res)
+{
+ mdiobus_free(*(struct mii_bus **)res);
+}
+
+static int devm_mdiobus_match(struct device *dev, void *res, void *data)
+{
+ struct mii_bus **r = res;
Please insert an empty line after declaration.
+ if (!r || !*r) {
+ WARN_ON(!r || !*r);
Hm, why we need the duplicate check This condition is always true.
[...]
+/**
+ * devm_mdiobus_free - Resource-managed mdiobus_free()
+ * @dev: Device this mii_bus belongs to
+ * @bus: the mii_bus associated with the device
+ *
+ * Free mii_bus allocated with devm_mdiobus_alloc().
+ */
+void devm_mdiobus_free(struct device *dev, struct mii_bus *bus)
+{
+ int rc;
+
+ rc = devres_release(dev, _devm_mdiobus_free,
+ devm_mdiobus_match, bus);
Please make this line start under 'dev', according to the networking
coding style.
[...]
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 2/2] net: davinci_mdio: use devm_* api
On 04-04-2014 17:40, Grygorii Strashko wrote:
Use devm_* API for memory allocation and to get device's clock
to simplify driver's code.
Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com
---
drivers/net/ethernet/ti/davinci_mdio.c | 21 -
1 file changed, 4 insertions(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/ti/davinci_mdio.c
b/drivers/net/ethernet/ti/davinci_mdio.c
index 0cca9de..f0f7128 100644
--- a/drivers/net/ethernet/ti/davinci_mdio.c
+++ b/drivers/net/ethernet/ti/davinci_mdio.c
[...]
@@ -425,16 +417,11 @@ static int davinci_mdio_remove(struct platform_device
*pdev)
if (data-bus) {
mdiobus_unregister(data-bus);
- mdiobus_free(data-bus);
}
Remove {} please, it's not needed anymore, according to
Documentation/CodingStyle.
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 06/62] ARM: davinci: export da8xx_syscfg0_base
Hello. On 03/20/2014 09:22 PM, Arnd Bergmann wrote: Yes, it does. The issue is that the PHY code is different in MUSB and OHCI drivers. I don't think the PHY driver knows what device binds to it. At least in the DT case, it can get that information from DT, when you set #phy-cells=1. I don't know how it would be done for platform_data, but I assume one could find a way too. #phy-cells is only defined for drivers/phy/ bindings IIRC, and I was thinking a drivers/usb/phy/ driver so far. Probably you have a point and we should go for the generic PHY framework instead. I'm just not very familiar with it... Arnd WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 06/62] ARM: davinci: export da8xx_syscfg0_base
On 03/20/2014 07:20 PM, Sergei Shtylyov wrote: The syscon (system controller) framework helps share a set of registers across multiple drivers. If all accesses to the CFGCHIP2 register are in a single PHY driver, we wouldn't need it. Where can I find it in the kernel tree? Found it. WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 06/62] ARM: davinci: export da8xx_syscfg0_base
On 03/19/2014 10:29 PM, Arnd Bergmann wrote: The ohci-da8xx driver uses the DA8XX_SYSCFG0_VIRT macro to access the CFGCHIP2 register for controlling its PHY. The macro in turn relies on the da8xx_syscfg0_base global variable. Since the OHCI driver can be a loadable module, this requires the symbol to be exported from platform code. Signed-off-by: Arnd Bergmann a...@arndb.de Cc: Sekhar Nori nsek...@ti.com Cc: Kevin Hilman khil...@deeprootsystems.com Cc: davinci-linux-open-source@linux.davincidsp.com --- arch/arm/mach-davinci/devices-da8xx.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c index 0486cdf..4da868a 100644 --- a/arch/arm/mach-davinci/devices-da8xx.c +++ b/arch/arm/mach-davinci/devices-da8xx.c @@ -66,6 +66,7 @@ #define DA850_DMA_MMCSD1_TX EDMA_CTLR_CHAN(1, 29) void __iomem *da8xx_syscfg0_base; +EXPORT_SYMBOL_GPL(da8xx_syscfg0_base); /* used by OHCI_HCD */ I have submitted such patch years ago and it was turned down. :-) WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 06/62] ARM: davinci: export da8xx_syscfg0_base
On 03/19/2014 11:21 PM, Arnd Bergmann wrote: The ohci-da8xx driver uses the DA8XX_SYSCFG0_VIRT macro to access the CFGCHIP2 register for controlling its PHY. The macro in turn relies on the da8xx_syscfg0_base global variable. Since the OHCI driver can be a loadable module, this requires the symbol to be exported from platform code. Signed-off-by: Arnd Bergmann a...@arndb.de Cc: Sekhar Nori nsek...@ti.com Cc: Kevin Hilman khil...@deeprootsystems.com Cc: davinci-linux-open-source@linux.davincidsp.com --- arch/arm/mach-davinci/devices-da8xx.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c index 0486cdf..4da868a 100644 --- a/arch/arm/mach-davinci/devices-da8xx.c +++ b/arch/arm/mach-davinci/devices-da8xx.c @@ -66,6 +66,7 @@ #define DA850_DMA_MMCSD1_TX EDMA_CTLR_CHAN(1, 29) void __iomem *da8xx_syscfg0_base; +EXPORT_SYMBOL_GPL(da8xx_syscfg0_base); /* used by OHCI_HCD */ I have submitted such patch years ago and it was turned down. I also had a pleasure of completing implementation of this driver and submitting it back in 2009 when I was working for MontaVista. :-) The question is whether there is anyone who would do this properly. Nobody cares, it seems. As you can imagine, I stopped to care enough after being switched to other projects. Both the OHCI and MUSB drivers use exactly one register (CFGCHIP2) The idea at the time was to just ioremap() this register but I was not very eager. There was no USB PHY driver subsystem yet. to control the clock, phy Note that it only controls PHY clock (PLL) which could be covered by an USB PHY driver. and host/gadget mode switch. The main mode the USB 2.0 PHY should function now is OTG. The host/gadged modes are forced overrides of the ID pin. Unfortunately, the board code has to force host mode when host-only driver config is selected (these MUSB's host/gadget only modes were removed at one point but got reintroduced again). In the modern world, we'd probably want to have a clock driver and Not sure about the clock driver... a phy driver for these, Yes, that's what the MUSB maintainer wants too. The issue is the driver should somehow differ which USB controller it's bound too and do different things depending on that (at least that's how it looks now). I'm not sure it's possible, so probably should be rethought. based on a syscon driver. I don't know what syscon is... In all honesty I don't see that happening on davinci. I don't think people use USB 1.1 controllers these days, especially when there is USB 2.0 in the same SoC. For MUSB, there were recent attempt to get the DA8xx driver out of the broken state but it was turned down as well, IIRC, since it didn't offer a PHY driver. A somewhat better approach would be to export a set of exported functions to access the one register from the platform, e.g. u32 da8xx_cfgchip2_get(void); void da8xx_cfgchip2_set(u32); That interface would still be a bit ugly, but much better than what we have today, and easy to implement. I'm leaving this idea to Sekhar... Arnd WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 0/5] ARM: davinci: tnetv107x removal
Hello. On 02/26/2014 04:46 PM, Arnd Bergmann wrote: The five patches are completely independent of one another, and applying them out of order is fine since we just want to remove the code. However, I'm looking for an Ack from Cyril Chemparathy and Sekhar Nori first, to be sure we won't need this code in the future. Kevin Hilman has already mentioned that he sees no reason to keep this code. Hmm, apparently Cyril is no longer at TI. If anyone has his current email address and thinks he might have an opinion, could you forward the original email? Done. Fished his Gmail address from LinkedIn. Arnd WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 0/2] DT: net: davinci_emac: couple more properties actually optional
Hello. On 01/29/2014 10:43 AM, David Miller wrote: Though described as required, couple more properties in the DaVinci EMAC binding are actually optional, as the driver will happily function without them. The patchset is against DaveM's 'net.git' tree this time. [1/2] DT: net: davinci_emac: ti,davinci-rmii-en property is actually optional [2/2] DT: net: davinci_emac: ti,davinci-no-bd-ram property is actually optional Series applied with the has/have thing fixed. Thanks. Thank you! Unfortunately, this driver presents a bad example of DT bindings overall (caused in its turn by a misuse of the platform data for the EMAC type differing instead of the platform device IDs). WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH 1/2] DT: net: davinci_emac: ti, davinci-rmii-en property is actually optional
Though described as required, the ti,davinci-rmii-en property for the DaVinci
EMAC binding seems actually optional, as the driver should happily work without
it; the property is not specified either in the example device node or in the
actual EMAC device node for DA850 device tree, only AM3517 one.
While at it, document the property better...
Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com
---
Actually I think this property should have been boolean...
Documentation/devicetree/bindings/net/davinci_emac.txt |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: net/Documentation/devicetree/bindings/net/davinci_emac.txt
===
--- net.orig/Documentation/devicetree/bindings/net/davinci_emac.txt
+++ net/Documentation/devicetree/bindings/net/davinci_emac.txt
@@ -10,7 +10,6 @@ Required properties:
- ti,davinci-ctrl-mod-reg-offset: offset to control module register
- ti,davinci-ctrl-ram-offset: offset to control module ram
- ti,davinci-ctrl-ram-size: size of control module ram
-- ti,davinci-rmii-en: use RMII
- ti,davinci-no-bd-ram: has the emac controller BD RAM
- interrupts: interrupt mapping for the davinci emac interrupts sources:
4 sources: Receive Threshold Interrupt
@@ -22,6 +21,7 @@ Optional properties:
- phy-handle: Contains a phandle to an Ethernet PHY.
If absent, davinci_emac driver defaults to 100/FULL.
- local-mac-address : 6 bytes, mac address
+- ti,davinci-rmii-en: 1 byte, 1 means use RMII
Example (enbw_cmc board):
eth0: emac@1e2 {
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH 2/2] DT: net: davinci_emac: ti, davinci-no-bd-ram property is actually optional
The ti,davinci-no-bd-ram property for the DaVinci EMAC binding simply can't be
required one, as it's boolean (which means it's absent if false).
While at it, document the property better...
Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com
---
Documentation/devicetree/bindings/net/davinci_emac.txt |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: net/Documentation/devicetree/bindings/net/davinci_emac.txt
===
--- net.orig/Documentation/devicetree/bindings/net/davinci_emac.txt
+++ net/Documentation/devicetree/bindings/net/davinci_emac.txt
@@ -10,7 +10,6 @@ Required properties:
- ti,davinci-ctrl-mod-reg-offset: offset to control module register
- ti,davinci-ctrl-ram-offset: offset to control module ram
- ti,davinci-ctrl-ram-size: size of control module ram
-- ti,davinci-no-bd-ram: has the emac controller BD RAM
- interrupts: interrupt mapping for the davinci emac interrupts sources:
4 sources: Receive Threshold Interrupt
Receive Interrupt
@@ -22,6 +21,7 @@ Optional properties:
If absent, davinci_emac driver defaults to 100/FULL.
- local-mac-address : 6 bytes, mac address
- ti,davinci-rmii-en: 1 byte, 1 means use RMII
+- ti,davinci-no-bd-ram: boolean, does EMAC has BD RAM?
Example (enbw_cmc board):
eth0: emac@1e2 {
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 2/2] DT: net: davinci_emac: ti, davinci-no-bd-ram property is actually optional
Hello. On 01/28/2014 02:49 AM, Sergei Shtylyov wrote: The ti,davinci-no-bd-ram property for the DaVinci EMAC binding simply can't be required one, as it's boolean (which means it's absent if false). While at it, document the property better... Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com --- Documentation/devicetree/bindings/net/davinci_emac.txt |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Index: net/Documentation/devicetree/bindings/net/davinci_emac.txt === --- net.orig/Documentation/devicetree/bindings/net/davinci_emac.txt +++ net/Documentation/devicetree/bindings/net/davinci_emac.txt @@ -10,7 +10,6 @@ Required properties: - ti,davinci-ctrl-mod-reg-offset: offset to control module register - ti,davinci-ctrl-ram-offset: offset to control module ram - ti,davinci-ctrl-ram-size: size of control module ram -- ti,davinci-no-bd-ram: has the emac controller BD RAM - interrupts: interrupt mapping for the davinci emac interrupts sources: 4 sources: Receive Threshold Interrupt Receive Interrupt @@ -22,6 +21,7 @@ Optional properties: If absent, davinci_emac driver defaults to 100/FULL. - local-mac-address : 6 bytes, mac address - ti,davinci-rmii-en: 1 byte, 1 means use RMII +- ti,davinci-no-bd-ram: boolean, does EMAC has BD RAM? Too hasty, s/has/have/. Do I need to resend or this could be fixed when applying? WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH] DT: net: davinci_emac: phy-handle property is actually optional
Though described as required, the phy-handle property for the DaVinci EMAC binding is actually optional, as the driver will happily function without it, assuming 100/FULL link; the property is not specified either in the example device node, or in the actual EMAC device nodes for DA850 and AM3517 device trees. Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com --- The patch is against DaveM's 'net-next.git' repo. Though being a fix, it does not seem important enough for 'net.git' repo at this time. Not sure if it should be considered for the stable kernels... Documentation/devicetree/bindings/net/davinci_emac.txt |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Index: renesas/Documentation/devicetree/bindings/net/davinci_emac.txt === --- renesas.orig/Documentation/devicetree/bindings/net/davinci_emac.txt +++ renesas/Documentation/devicetree/bindings/net/davinci_emac.txt @@ -12,8 +12,6 @@ Required properties: - ti,davinci-ctrl-ram-size: size of control module ram - ti,davinci-rmii-en: use RMII - ti,davinci-no-bd-ram: has the emac controller BD RAM -- phy-handle: Contains a phandle to an Ethernet PHY. - if not, davinci_emac driver defaults to 100/FULL - interrupts: interrupt mapping for the davinci emac interrupts sources: 4 sources: Receive Threshold Interrupt Receive Interrupt @@ -21,6 +19,8 @@ Required properties: Miscellaneous Interrupt Optional properties: +- phy-handle: Contains a phandle to an Ethernet PHY. + If absent, davinci_emac driver defaults to 100/FULL. - local-mac-address : 6 bytes, mac address Example (enbw_cmc board): ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH] pinctrl: pinctrl-single: Convert to devm_ioremap_resource()
Hello.
On 27-08-2013 11:05, Vishwanathrao Badarkhe, Manish wrote:
From: Vishwanathrao Badarkhe, Manish manish...@ti.com
Convert devm_request_mem_region() and devm_ioremap() to
devm_ioremap_resource() which provides more consistent
error handling to manage resource.
Signed-off-by: Vishwanathrao Badarkhe, Manish manish...@ti.com
---
:100644 100644 7323cca... b0fef18... M drivers/pinctrl/pinctrl-single.c
drivers/pinctrl/pinctrl-single.c | 21 +++--
1 file changed, 3 insertions(+), 18 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
index 7323cca..b0fef18 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -1556,24 +1556,9 @@ static int pcs_probe(struct platform_device *pdev)
pinctrl-single,bit-per-mux);
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res) {
- dev_err(pcs-dev, could not get resource\n);
- return -ENODEV;
- }
-
- pcs-res = devm_request_mem_region(pcs-dev, res-start,
- resource_size(res), DRIVER_NAME);
- if (!pcs-res) {
Is pcs-res used anywhere else?
- dev_err(pcs-dev, could not get mem_region\n);
- return -EBUSY;
- }
-
- pcs-size = resource_size(pcs-res);
Is pcs-size used anywhere else?
- pcs-base = devm_ioremap(pcs-dev, pcs-res-start, pcs-size);
- if (!pcs-base) {
- dev_err(pcs-dev, could not ioremap\n);
- return -ENODEV;
- }
+ pcs-base = devm_ioremap_resource(pcs-dev, res);
+ if (IS_ERR(pcs-base))
+ return PTR_ERR(pcs-base);
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v5 1/2] ARM: davinci: da850: add DT node for ethernet
Hello.
On 08/16/2013 06:11 PM, Lad, Prabhakar wrote:
From: Lad, Prabhakar prabhakar.cse...@gmail.com
Add ethernet device tree node information and pinmux for mii to da850 by
providing interrupt details and local mac address.
Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
---
arch/arm/boot/dts/da850-evm.dts |5 +
arch/arm/boot/dts/da850.dtsi| 30 ++
2 files changed, 35 insertions(+)
diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
index 1f8cfdd..44f3fbc 100644
--- a/arch/arm/boot/dts/da850-evm.dts
+++ b/arch/arm/boot/dts/da850-evm.dts
@@ -96,6 +96,11 @@
pinctrl-0 = mdio_pins;
bus_freq = 220;
};
+ ethernet: emac@1e2 {
Why label the node in the board file too?
+ status = okay;
+ pinctrl-names = default;
+ pinctrl-0 = mii_pins;
+ };
};
nand_cs3@6200 {
status = okay;
diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index d5138b4..a5d30d9 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
[...]
@@ -226,6 +242,20 @@
#size-cells = 0;
reg = 0x224000 0x1000;
};
+ ethernet: emac@1e2 {
I said node should be named ethernet, not labelled. You're the second
person that doesn't know the difference between the two. :-)
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v4 4/5] ARM: davinci: da850: add DT node for eth0.
Hello.
On 15-08-2013 10:01, Lad, Prabhakar wrote:
From: Lad, Prabhakar prabhakar.cse...@gmail.com
Add eth0 device tree node information and pinmux for mii to da850 by
providing interrupt details and local mac address of eth0.
Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
[...]
@@ -226,6 +242,20 @@
#size-cells = 0;
reg = 0x224000 0x1000;
};
+ eth0: emac@1e2 {
According to the ePAPR spec[1] section 2.2.2, the node should be named
ethernet.
[1] http://www.power.org/resources/downloads/Power_ePAPR_APPROVED_v1.0.pdf
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v4 5/5] ARM: davinci: da850: add OF_DEV_AUXDATA entry for eth0.
Hello.
On 15-08-2013 10:01, Lad, Prabhakar wrote:
From: Lad, Prabhakar prabhakar.cse...@gmail.com
Add OF_DEV_AUXDATA for eth0 driver in da850 board dt
file to use emac clock.
Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
[...]
diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
index d172563..caa9202 100644
--- a/arch/arm/mach-davinci/da8xx-dt.c
+++ b/arch/arm/mach-davinci/da8xx-dt.c
@@ -44,6 +44,9 @@ static struct of_dev_auxdata da850_auxdata_lookup[]
__initdata = {
OF_DEV_AUXDATA(ns16550a, 0x01d0c000, serial8250.1, NULL),
OF_DEV_AUXDATA(ns16550a, 0x01d0d000, serial8250.2, NULL),
OF_DEV_AUXDATA(ti,davinci_mdio, 0x01e24000, davinci_mdio.0, NULL),
+ OF_DEV_AUXDATA(ti,davinci-dm6467-emac, 0x01e2, davinci_emac.1,
+ NULL),
+
Empty line hardly needed here.
{}
};
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v3 2/9] media: davinci: vpif: Convert to devm_* api
Hello.
On 26-05-2013 16:00, Prabhakar Lad wrote:
From: Lad, Prabhakar prabhakar.cse...@gmail.com
Use devm_ioremap_resource instead of reques_mem_region()/ioremap().
This ensures more consistent error values and simplifies error paths.
Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
---
drivers/media/platform/davinci/vpif.c | 27 ---
1 files changed, 4 insertions(+), 23 deletions(-)
diff --git a/drivers/media/platform/davinci/vpif.c
b/drivers/media/platform/davinci/vpif.c
index 761c825..f857d8f 100644
--- a/drivers/media/platform/davinci/vpif.c
+++ b/drivers/media/platform/davinci/vpif.c
[...]
@@ -421,23 +419,12 @@ EXPORT_SYMBOL(vpif_channel_getfid);
static int vpif_probe(struct platform_device *pdev)
{
- int status = 0;
+ static struct resource *res;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
[...]
+ vpif_base = devm_request_and_ioremap(pdev-dev, res);
No, don't use this deprecated funtion please. Undo to
devm_ioremap_resource().
+ if (IS_ERR(vpif_base))
NAK, devm_request_and_ioremap() doesn't rethrn error cpdes, only
NULL. BTW, it's implemented via a call to devm_ioremap_resource() now.
Is it so hard to look at the code that you've calling?
+ return PTR_ERR(vpif_base);
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v3 4/9] media: davinci: vpif_capture: move the freeing of irq and global variables to remove()
Hello.
On 26-05-2013 16:00, Prabhakar Lad wrote:
From: Lad, Prabhakar prabhakar.cse...@gmail.com
Ideally the freeing of irq's and the global variables needs to be
done in the remove() rather than module_exit(), this patch moves
the freeing up of irq's and freeing the memory allocated to channel
objects to remove() callback of struct platform_driver.
Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
---
drivers/media/platform/davinci/vpif_capture.c | 31 ++--
1 files changed, 13 insertions(+), 18 deletions(-)
diff --git a/drivers/media/platform/davinci/vpif_capture.c
b/drivers/media/platform/davinci/vpif_capture.c
index caaf4fe..f8b7304 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -2225,17 +2225,29 @@ vpif_int_err:
*/
static int vpif_remove(struct platform_device *device)
{
- int i;
+ struct platform_device *pdev;
struct channel_obj *ch;
+ struct resource *res;
+ int irq_num, i = 0;
+
+ pdev = container_of(vpif_dev, struct platform_device, dev);
Why you need this if you should be already called with the correct
platform device?
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 02/11] gpio: davinci: coding style correction
Hello.
On 22-05-2013 11:10, Philip Avinash wrote:
1. Corrects coding and commenting styles
2. Variables name change to meaningful name
3. Remove unnecessary variable usage
4. Add BINTEN macro definition
Signed-off-by: Philip Avinash avinashphi...@ti.com
---
drivers/gpio/gpio-davinci.c | 182 +--
1 file changed, 89 insertions(+), 93 deletions(-)
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 17df6db..d308955 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
[...]
@@ -31,10 +31,11 @@ struct davinci_gpio_regs {
u32 intstat;
};
+#define BINTEN 0x08 /* GPIO Interrupt Per-Bank Enable Register */
Empty line needed here.
#define chip2controller(chip) \
container_of(chip, struct davinci_gpio_controller, chip)
[...]
@@ -98,8 +94,8 @@ static int davinci_direction_in(struct gpio_chip *chip,
unsigned offset)
return __davinci_direction(chip, offset, false, 0);
}
-static int
-davinci_direction_out(struct gpio_chip *chip, unsigned offset, int value)
+static int davinci_direction_out(struct gpio_chip *chip, unsigned offset,
+ int value)
This line should be aligned under the next character after (.
[...]
@@ -113,22 +109,22 @@ davinci_direction_out(struct gpio_chip *chip, unsigned
offset, int value)
[...]
/*
* Assuming the pin is muxed as a gpio output, set its output value.
*/
-static void
-davinci_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
+static void davinci_gpio_set(struct gpio_chip *chip, unsigned offset,
+ int value)
Same here.
[...]
@@ -368,16 +363,16 @@ static int __init davinci_gpio_irq_setup(void)
[...]
for (gpio = 0, bank = 0; gpio ngpio; bank++, gpio += 32) {
- chips[bank].chip.to_irq = gpio_to_irq_banked;
- chips[bank].irq_base = soc_info-gpio_unbanked
- ? -EINVAL
- : (soc_info-intc_irq_num + gpio);
+ ctlrs[bank].chip.to_irq = gpio_to_irq_banked;
+ ctlrs[bank].irq_base = soc_info-gpio_unbanked ?
+ -EINVAL : (soc_info-intc_irq_num + gpio);
() not needed here.
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH] ARM: davinci: dma: Convert to devm_* api
Hello.
On 16-05-2013 10:58, Lad Prabhakar wrote:
From: Lad, Prabhakar prabhakar.cse...@gmail.com
Use devm_ioremap_resource instead of reques_mem_region()/ioremap() and
devm_request_irq() instead of request_irq().
This ensures more consistent error values and simplifies error paths.
Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
---
NOte:- Boot tested on Logic-PD OMAP-L138 EVM
arch/arm/mach-davinci/dma.c | 63 --
1 files changed, 24 insertions(+), 39 deletions(-)
diff --git a/arch/arm/mach-davinci/dma.c b/arch/arm/mach-davinci/dma.c
index 45b7c71..aeda496 100644
--- a/arch/arm/mach-davinci/dma.c
+++ b/arch/arm/mach-davinci/dma.c
[...]
@@ -1422,25 +1421,16 @@ static int __init edma_probe(struct platform_device
*pdev)
found = 1;
}
- len[j] = resource_size(r[j]);
-
- r[j] = request_mem_region(r[j]-start, len[j],
- dev_name(pdev-dev));
- if (!r[j]) {
- status = -EBUSY;
- goto fail1;
- }
-
- edmacc_regs_base[j] = ioremap(r[j]-start, len[j]);
- if (!edmacc_regs_base[j]) {
+ edmacc_regs_base[j] = devm_ioremap_resource(pdev-dev, r[j]);
+ if (IS_ERR(edmacc_regs_base[j])) {
status = -EBUSY;
And you call that more consistent error values? Why not:
status = PTR_ERR(edmacc_regs_base[j]);
edma_cc[j] = kzalloc(sizeof(struct edma), GFP_KERNEL);
Maybe it's worth using devm_kzalloc() too?
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 1/5] davinci: net: cpdma: remove unwanted header file incusion and sort thme alphabetically
Hello. On 16-05-2013 11:30, Lad Prabhakar wrote: s/incusion/inclusion/, s/thme/them/ in the subject. Though instead of them it would be better to write headers. From: Lad, Prabhakar prabhakar.cse...@gmail.com Changelog won't hurt here... which unwanted #include's you are removing and why are they unwanted? Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com --- drivers/net/ethernet/ti/davinci_cpdma.c | 10 +++--- 1 files changed, 3 insertions(+), 7 deletions(-) diff --git a/drivers/net/ethernet/ti/davinci_cpdma.c b/drivers/net/ethernet/ti/davinci_cpdma.c index 49dfd59..f7ce97f 100644 --- a/drivers/net/ethernet/ti/davinci_cpdma.c +++ b/drivers/net/ethernet/ti/davinci_cpdma.c @@ -12,15 +12,11 @@ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU General Public License for more details. */ -#include linux/kernel.h -#include linux/spinlock.h -#include linux/device.h + +#include linux/delay.h +#include linux/dma-mapping.h #include linux/module.h #include linux/slab.h -#include linux/err.h -#include linux/dma-mapping.h -#include linux/io.h -#include linux/delay.h #include davinci_cpdma.h WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 2/5] net: davinci_emac: remove unwanted header inclusion and sort the alphabetically
On 16-05-2013 11:30, Lad Prabhakar wrote: From: Lad, Prabhakar prabhakar.cse...@gmail.com This patch removes unwanted header inclusion Why are they unwanted? and sorts them alphabetically In the subject you typed the instead of them. Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 5/5] net: davinci_mdio: trivial cleanup
On 16-05-2013 11:30, Lad Prabhakar wrote: From: Lad, Prabhakar prabhakar.cse...@gmail.com remove unwanted header inclusion and sort the alphabetically s/the/them/. also guard the davinci_mdio_of_mtable table and davinci_mdio_probe_dt() with CONFIG_OF. Saying also in the changelog is often a good sign there should be one more patch -- as in this case. Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH] media: davinci: vpbe: fix layer availability for NV12 format
Hello.
On 03-05-2013 13:53, Prabhakar Lad wrote:
From: Lad, Prabhakar prabhakar.cse...@gmail.com
For NV12 format, even if display data is single image,
both VIDWIN0 and VIDWIN1 parameters must be used. The start
address of Y data plane and C data plane is configured in
VIDEOWIN0ADH/L and VIDEOWIN1ADH/L respectively.
cuurently only one layer was requested, which is suffice
for yuv422, but for yuv420(NV12) two layers are required and
fix the same by requesting for other layer if pix fmt is NV12
during set_fmt.
Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
---
drivers/media/platform/davinci/vpbe_display.c | 16
1 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/drivers/media/platform/davinci/vpbe_display.c
b/drivers/media/platform/davinci/vpbe_display.c
index 0341dcc..f2ee07b 100644
--- a/drivers/media/platform/davinci/vpbe_display.c
+++ b/drivers/media/platform/davinci/vpbe_display.c
@@ -922,6 +922,22 @@ static int vpbe_display_s_fmt(struct file *file, void
*priv,
other video window */
layer-pix_fmt = *pixfmt;
+ if (pixfmt-pixelformat == V4L2_PIX_FMT_NV12
+ cpu_is_davinci_dm365()) {
cpu_is_*() shouldn't be used in the drivers.
+ struct vpbe_layer *otherlayer;
+
+ otherlayer = _vpbe_display_get_other_win_layer(disp_dev, layer);
+ /* if other layer is available, only
+ * claim it, do not configure it
+ */
+ ret = osd_device-ops.request_layer(osd_device,
+ otherlayer-layer_info.id);
+ if (ret 0) {
+ v4l2_err(vpbe_dev-v4l2_dev,
+Display Manager failed to allocate layer\n);
+ return -EBUSY;
+ }
+ }
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH] media: davinci: vpbe: align the buffers size to page page size boundary
On 16-04-2013 15:10, Prabhakar lad wrote: From: Lad, Prabhakar prabhakar.cse...@gmail.com with recent commit with id 068a0df76023926af958a336a78bef60468d2033 Please also specify the summary line of that commit in parens (or however you like). which adds add length check for mmap, the application were failing to mmap the buffers. This patch aligns the the buffer size to page size boundary for both capture and display driver so the it pass the check. Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com Cc: Hans Verkuil hans.verk...@cisco.com Cc: Mauro Carvalho Chehab mche...@redhat.com WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2] media: davinci: vpif: align the buffers size to page page size boundary
Hello. On 16-04-2013 14:54, Prabhakar lad wrote: From: Lad, Prabhakar prabhakar.cse...@gmail.com with recent commit with id 068a0df76023926af958a336a78bef60468d2033 Please also specify the summary line of that commit in parens (or however you like). which adds add length check for mmap, the application were failing to mmap the buffers. This patch aligns the the buffer size to page size boundary for both capture and display driver so the it pass the check. Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com Cc: Hans Verkuil hans.verk...@cisco.com Cc: Mauro Carvalho Chehab mche...@redhat.com WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v9 4/6] ARM: davinci: Add a remoteproc driver implementation for OMAP-L13x DSP
Hello.
On 09-04-2013 3:55, Tivy, Robert wrote:
+static int da8xx_rproc_probe(struct platform_device *pdev)
+{
+ struct device *dev = pdev-dev;
+ struct da8xx_rproc *drproc;
+ struct rproc *rproc;
+ struct irq_data *irq_data;
+ struct resource *bootreg_res;
+ struct resource *chipsig_res;
+ struct clk *dsp_clk;
+ void __iomem *chipsig;
+ void __iomem *bootreg;
+ int irq;
+ int ret;
+
[...]
+ bootreg = devm_request_and_ioremap(dev, bootreg_res);
+ if (!bootreg) {
+ dev_err(dev, unable to map boot register\n);
+ return -EADDRNOTAVAIL;
+ }
+
+ chipsig = devm_request_and_ioremap(dev, chipsig_res);
I suggest that you use more modern (yes, already a newer interface
:-)
devm_ioremap_resource() instead -- it returns the error code (as a
pointer)
in case of error, and it certainly doesn't require you to print error
messages.
Thanks, will do.
I appreciate the notice of a more modern function, it's really tough to keep up
with the flurry of activity to the kernel.
Regarding this change, should the code use
return PTR_ERR(bootreg);
or
return PTR_RET(bootreg);
The former, to avoid duplicate IS_ERR() check.
I ask because PTR_ERR() returns 'long' whereas PTR_RET() returns 'int' (and probe returns
'int'), but I see that the majority of existing code uses return PTR_ERR() in
probe functions.
But PTR_RET() uses PTR_ERR() internally anyway.
+ if (!chipsig) {
+ dev_err(dev, unable to map CHIPSIG register\n);
+ return -EADDRNOTAVAIL;
+ }
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v9 4/6] ARM: davinci: Add a remoteproc driver implementation for OMAP-L13x DSP
Hello.
On 04/07/2013 05:02 PM, Ohad Ben-Cohen wrote:
+static int da8xx_rproc_probe(struct platform_device *pdev)
+{
+ struct device *dev = pdev-dev;
+ struct da8xx_rproc *drproc;
+ struct rproc *rproc;
+ struct irq_data *irq_data;
+ struct resource *bootreg_res;
+ struct resource *chipsig_res;
+ struct clk *dsp_clk;
+ void __iomem *chipsig;
+ void __iomem *bootreg;
+ int irq;
+ int ret;
+
[...]
+ bootreg = devm_request_and_ioremap(dev, bootreg_res);
+ if (!bootreg) {
+ dev_err(dev, unable to map boot register\n);
+ return -EADDRNOTAVAIL;
+ }
+
+ chipsig = devm_request_and_ioremap(dev, chipsig_res);
I suggest that you use more modern (yes, already a newer interface :-)
devm_ioremap_resource() instead -- it returns the error code (as a pointer)
in case of error, and it certainly doesn't require you to print error
messages.
+ if (!chipsig) {
+ dev_err(dev, unable to map CHIPSIG register\n);
+ return -EADDRNOTAVAIL;
+ }
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v8 3/7] ARM: davinci: Add support for configuring DA8XX_REMOTEPROC
Hello. On 09-03-2013 2:41, Robert Tivy wrote: Also fix REMOTEPROC config to select FW_LOADER (instead of FW_CONFIG). Signed-off-by: Robert Tivy rt...@ti.com --- drivers/remoteproc/Kconfig | 26 +- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig index 96ce101..21d04f1 100644 --- a/drivers/remoteproc/Kconfig +++ b/drivers/remoteproc/Kconfig [...] @@ -41,4 +41,28 @@ config STE_MODEM_RPROC This can be either built-in or a loadable module. If unsure say N. +config DA8XX_REMOTEPROC + tristate DA830/OMAPL137 DA850/OMAPL138 remoteproc support (EXPERIMENTAL) + depends on ARCH_DAVINCI_DA8XX + select REMOTEPROC + select RPMSG + select CMA + default n + help + Say y here to support DA830/OMAPL137 DA850/OMAPL138 remote Naming nit: they're OMAP-L13x. Here and above. WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: DM6446 using multiple USB devices
Hello.
On 06-03-2013 8:45, B, Ravi wrote:
All bulk endpoint transfers are sheduled using the single MUSB EP 1,
AFAIR.
2. Is there a software work around (Interrupt endpoint scheduling)
that works with devices that need Interrupt EPs? If so what kernel
version do I need?
Interrupt endpoint scheduling was implemented for very early v2.6.10
based MV kernels that TI used to ship. Recently Ravi up-ported that to
v3.3 but was only tested it on DA850 as per my knowledge. Here is the
link:
http://arago-project.org/git/projects/?p=linux-davinci.git;a=commit;h=0795c14aa91650d778a27fe7b2ef23e2d9ff8c89
This code seems to have the same mistake I fixed for 2.6.18 MV release
--
it tries to schedule several URBs concurrently on the same endpoint. It
seems
TI engineers have learned nothing from my work. :-(
Is there different approach to this ? Can you explain in detail ?
You can't execute several URBs to one endpoint in parallel, trying to
interleave them -- that's totally incorrect. Please find the sources of
2.6.18 based TI PSP and compare with your 3.3 code. Although, here's the
changes I did to the original interrupt scheduling patch (extracted from so
called patch #47) back then:
diff -u linux-2.6.18/drivers/usb/musb/musb_host.c
linux-2.6.18/drivers/usb/musb/musb_host.c
--- linux-2.6.18/drivers/usb/musb/musb_host.c
+++ linux-2.6.18/drivers/usb/musb/musb_host.c
@@ -269,6 +269,11 @@
else
musb-intr_hold = 1;
}
+
+ qh-interval = urb-interval;
+ if ((musb-port1_status USB_PORT_STAT_HIGH_SPEED)
+ urb-dev-speed != USB_SPEED_HIGH)
+ qh-interval *= 8;
}
/* FALLTHROUGH */
#endif /* CONFIG_MUSB_SCHEDULE_INTR_EP */
@@ -341,8 +346,6 @@
__releases(musb-lock)
__acquires(musb-lock)
{
- struct musb_qh *qh = urb-hcpriv;
-
if ((urb-transfer_flags URB_SHORT_NOT_OK)
(urb-actual_length urb-transfer_buffer_length)
status == 0
@@ -350,10 +353,8 @@
status = -EREMOTEIO;
spin_lock(urb-lock);
-
urb-hcpriv = NULL;
- if (urb-status == -EINPROGRESS ||
- (musb-intr_ep == qh-hw_ep urb-status == -EPROTO))
+ if (urb-status == -EINPROGRESS)
urb-status = status;
spin_unlock(urb-lock);
@@ -507,9 +508,10 @@
static void musb_host_intr_schedule(struct musb *musb)
{
struct musb_hw_ep *hw_ep = musb-intr_ep;
+ void __iomem*epio = hw_ep-regs;
struct urb *purb, *hurb = NULL;
struct musb_qh *pqh, *hqh = NULL;
- u16 csr = 0;
+ u16 csr;
/*
* Hold the current interrupt request until the IN token is sent to
@@ -517,50 +519,44 @@
* for scheduling other device's interrupt requests.
*/
if (musb-intr_hold != 0 --musb-intr_hold == 0) {
- csr = musb_readw(hw_ep-regs, MUSB_RXCSR);
-
+ csr = musb_readw(epio, MUSB_RXCSR);
csr = ~(MUSB_RXCSR_H_ERROR | MUSB_RXCSR_DATAERROR |
MUSB_RXCSR_H_RXSTALL | MUSB_RXCSR_H_REQPKT);
/* Avoid clearing RXPKTRDY */
- musb_writew(hw_ep-regs, MUSB_RXCSR, csr | MUSB_RXCSR_RXPKTRDY);
+ musb_writew(epio, MUSB_RXCSR, csr | MUSB_RXCSR_RXPKTRDY);
}
- list_for_each_entry(pqh, musb-in_intr, ring)
- list_for_each_entry(purb, pqh-hep-urb_list, urb_list) {
-
- if (purb-number_of_packets)
- purb-number_of_packets--;
- /*
-* If a contention occurs in the same frame period
-* between several Interrupt requests expiring
-* then look for speed as the primary yardstick.
-* If they are of the same speed then look for the
-* lesser polling interval request.
-*/
- if (purb-number_of_packets = 0 !musb-intr_hold
- purb-status != -EPROTO) {
- if (hurb) {
- if (hurb-dev-speed ==
- purb-dev-speed) {
- if (hurb-interval =
- purb-interval)
- continue;
- } else if (hurb-dev-speed
- purb-dev-speed)
+ list_for_each_entry(pqh, musb-in_intr, ring) {
+ if (pqh-interval)
+ pqh-interval--;
+ /*
+
Re: DM6446 using multiple USB devices
Hello. On 05-03-2013 12:17, Sekhar Nori wrote: On 3/4/2013 11:32 PM, Danny Marsh wrote: I have read many posts and I am still unclear if there is a solution to our issue. We are using a custom DM6446 board running Linux git kernel 3.0.0 with a TUSB2046B http://www.ti.com/product/TUSB2046B hub. We are trying to hook up 2 usbnet devices (using the cdc_ethernet driver). Each device needs the following EPs. TUSB2046B http://www.ti.com/product/TUSB2046B - 1 Interrupt EP USB Device 1 - 1 Interrupt EP + 2 Bulk EPs USB Device 2 - 1 Interrupt EP + 2 Bulk EPs 1. Is there a hardware limitation in the DM6446 that prevents us from doing this? DM644x has 4 endpoints. 5, counting the control one. You are definitely shooting above that mark from this description. All bulk endpoint transfers are sheduled using the single MUSB EP 1, AFAIR. 2. Is there a software work around (Interrupt endpoint scheduling) that works with devices that need Interrupt EPs? If so what kernel version do I need? Interrupt endpoint scheduling was implemented for very early v2.6.10 based MV kernels that TI used to ship. Recently Ravi up-ported that to v3.3 but was only tested it on DA850 as per my knowledge. Here is the link: http://arago-project.org/git/projects/?p=linux-davinci.git;a=commit;h=0795c14aa91650d778a27fe7b2ef23e2d9ff8c89 This code seems to have the same mistake I fixed for 2.6.18 MV release -- it tries to schedule several URBs concurrently on the same endpoint. It seems TI engineers have learned nothing from my work. :-( WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: DM6446 using multiple USB devices
Hello. On 05-03-2013 12:17, Sekhar Nori wrote: Hi Danny, I will reply from my limited USB knowledge. I have copied Ravi who knows more, but is out sick ATM. Did you BCC him or did you just forget to CC him? I don't see his address in your mail. That's too bad -- I would like him to read my comment on his interrupt EP scheduling patch port to 3.3. Please forward my mail to him. WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH] watchdog: davinci_wdt: update to devm_* API
Hello.
On 07-02-2013 7:32, Kumar, Anil wrote:
Update the code to use devm_* API so that driver
core will manage resources.
Signed-off-by: Kumar, Anil anilkuma...@ti.com
---
This patch applies on top of v3.8-rc6.
Tested on da850 EVM.
:100644 100644 e8e8724... 6ad76a3... M drivers/watchdog/davinci_wdt.c
drivers/watchdog/davinci_wdt.c | 34 +-
1 files changed, 9 insertions(+), 25 deletions(-)
diff --git a/drivers/watchdog/davinci_wdt.c b/drivers/watchdog/davinci_wdt.c
index e8e8724..6ad76a3 100644
--- a/drivers/watchdog/davinci_wdt.c
+++ b/drivers/watchdog/davinci_wdt.c
[...]
@@ -213,49 +212,34 @@ static int davinci_wdt_probe(struct platform_device *pdev)
[...]
- size = resource_size(wdt_mem);
- if (!request_mem_region(wdt_mem-start, size, pdev-name)) {
- dev_err(dev, failed to get memory region\n);
- return -ENOENT;
- }
-
- wdt_base = ioremap(wdt_mem-start, size);
+ wdt_base = devm_request_and_ioremap(pdev-dev, wdt_mem);
if (!wdt_base) {
- dev_err(dev, failed to map memory region\n);
- release_mem_region(wdt_mem-start, size);
- wdt_mem = NULL;
+ dev_err(pdev-dev, ioremap failed\n);
return -ENOMEM;
Comment to devm_request_and_ioremap() suggest returning -EADDRNOTAVAIL
instead.
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH V2] i2c: davinci: update to devm_* API
Hello.
On 06-02-2013 15:22, Vishwanathrao Badarkhe, Manish wrote:
Update the code to use devm_* API so that driver
core will manage resources.
Signed-off-by: Vishwanathrao Badarkhe, Manish manish...@ti.com
---
Changes since V1:
- Rebase on top of v3.8-rc6 of linus tree.
- Apply devm operation on clk_get.
:100644 100644 6a0a553... da4e218... M drivers/i2c/busses/i2c-davinci.c
drivers/i2c/busses/i2c-davinci.c | 45 +++---
1 files changed, 13 insertions(+), 32 deletions(-)
diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 6a0a553..da4e218 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
[...]
@@ -699,22 +693,24 @@ static int davinci_i2c_probe(struct platform_device *pdev)
dev-pdata = davinci_i2c_platform_data_default;
}
- dev-clk = clk_get(pdev-dev, NULL);
+ dev-clk = devm_clk_get(pdev-dev, NULL);
if (IS_ERR(dev-clk)) {
r = -ENODEV;
goto err_free_mem;
}
clk_prepare_enable(dev-clk);
- dev-base = ioremap(mem-start, resource_size(mem));
+ dev-base = devm_request_and_ioremap(pdev-dev, mem);
if (!dev-base) {
r = -EBUSY;
Comment to devm_request_and_ioremap() suggests returning -EADDRNOTAVAIL on
failure. -EBUSY wasn't the right code even before this change, should have
been -ENOMEM.
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 2/2] ARM: davinci: da850: add wdt OF_DEV_AUXDATA entry
Hello.
On 04-02-2013 15:50, Sekhar Nori wrote:
Auxdata is not evm specific. This can instead be called da850_auxdata_lookup[].
Also, I dont think it is necessary to add auxdata in a separate patch
from dt nodes. So, I fixed these issues and came up with below patch. I
tested basic wdt reboot. reboot command is still broken (with or
without this patch). Can you please look at that?
Thanks,
Sekhar
8
From: Kumar, Anil anilkuma...@ti.com
Date: Thu, 24 Jan 2013 14:08:14 +0530
Subject: [PATCH 1/1] ARM: davinci: da850: add wdt DT node
Add da850 wdt DT node.
Signed-off-by: Kumar, Anil anilkuma...@ti.com
Signed-off-by: Sekhar Nori nsek...@ti.com
[...]
diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index 8dd15c0..2800090 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -88,6 +88,11 @@
19;
status = disabled;
};
+ wdt: wdt@1c21000 {
+ compatible = ti,davinci-wdt;
+ reg = 0x21000 0xfff;
Not 0x1000? This is region size, not upper limit.
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common
Hello. On 02/04/2013 06:41 PM, Felipe Balbi wrote: I guess to make the MUSB side simpler we would need musb-dma-engine glue to map dmaengine to the private MUSB API. Then we would have some starting point to also move inventra (and anybody else) to dmaengine API. Why? Inventra is a dedicated device's private DMA controller, why make universal DMA driver for it? because it doesn't make sense to support multiple DMA APIs. We can check from MUSB's registers if it was configured with Inventra DMA support and based on that we can register MUSB's own DMA Engine to dmaengine API. Hang on. This is one of the DMA implementations which is closely coupled with the USB and only the USB? If it is... I thought this had been discussed _extensively_ before. I thought the resolution on it was: 1. It would not use the DMA engine API. 2. It would not live in arch/arm. 3. It would be placed nearby the USB driver it's associated with. (1) because we don't use APIs just for the hell of it - think. Do we use the DMA engine API for PCI bus mastering ethernet controllers? No. Do we use it for PCI bus mastering SCSI controllers? No. Because the DMA is integral to the rest of the device. that's not really a fair comparison, however. MUSB is used with several DMA engines. The only DMA engine which is really coupled with MUSB is the Inventra DMA engine which comes as an optional feature to the IP. Many users have That's not quite true. At least CPPI 3.0 is coupled with MUSB as well. The programming interface is MUSB specific, and differs from that of the EMAC driver which also uses CPPI 3.0 (however, the DMA descriptor format is the same between both, IIRC). opted out of it. From the top of my head we have CPPI 3.x, CPPI 4.1, Inventra DMA, OMAP sDMA and ux500 DMA engines supported by the driver. Granted, CPPI 4.1 makes some assumptions about the fact that it's handling USB tranfers, What CPPI 4.1 code makes this assumptions? MUSB DMA driver? Then it's just natural. Generic CPPI 4.1 support code (as was posted for both mach-dacinci/ or common/ placement) made no such assumptions. but nevertheless, the IP can be, and in fact is, used with many different DMA engines and driver needs to cope with it. What IP, CPPI 4.1 or MUSB? Current DMA abstraction is quite poor, for example there's no way to compile support for multiple DMA engines. Code also makes certain, IMO unnecessary, assumptions about the underlying DMA engine (abstraction is poor, as said above but it we could follow MUSB's programming guide when it comes to programming DMA transfers). Don't know, I was quite content with the abstraction when writing CPPI 4.1 driver for MUSB... Considering all of the above, it's far better to use DMA engine and get rid of all the mess. In my eyes, getting rid of the mess doesn't justify breaking the rules that Russell formulated above. WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common
Hello. On 02/04/2013 07:47 PM, Felipe Balbi wrote: On Mon, Feb 04, 2013 at 08:36:38PM +0300, Sergei Shtylyov wrote: opted out of it. From the top of my head we have CPPI 3.x, CPPI 4.1, Inventra DMA, OMAP sDMA and ux500 DMA engines supported by the driver. Granted, CPPI 4.1 makes some assumptions about the fact that it's handling USB tranfers, What CPPI 4.1 code makes this assumptions? MUSB DMA driver? Then it's just HW makes the asumptions Not true at all. There is a separate set of registers (at offset 0) which copes with USB specifics, but CPPI 4.1 itself doesn't know about USB anything. It's just the way the driver was written that it used both sets of registers but this needs to be changed into more abstacted accesses to the USB-specific part, to cope with it being different on different platfroms, like AM35x. The driver as it was last posted, just needs rework now. but nevertheless, the IP can be, and in fact is, used with many different DMA engines and driver needs to cope with it. What IP, CPPI 4.1 or MUSB? MUSB Current DMA abstraction is quite poor, for example there's no way to compile support for multiple DMA engines. Code also makes certain, IMO unnecessary, assumptions about the underlying DMA engine (abstraction is poor, as said above but it we could follow MUSB's programming guide when it comes to programming DMA transfers). Don't know, I was quite content with the abstraction when writing CPPI 4.1 driver for MUSB... look closer. The whole: if (is_cppi()) foo(); else if (is_inventra()) bar(); else if (is_omap_sdma()) baz(); is bogus. That part -- yes. There were attempt to get rid of this, but without changing the DMA API. It was left halfway done after my only critical comment, IIRC. Will we ever see the continuation of this effort? Considering all of the above, it's far better to use DMA engine and get rid of all the mess. In my eyes, getting rid of the mess doesn't justify breaking the rules that Russell formulated above. MUSB is no PCI, there is no single, standard interface to the DMA engine (unlike Synopsys' dwc-usb3 and dwc-usb2, where we don't use DMA engine), every DMA engine comes with its own set of registers, its own programming model and so forth. Same can be said about PCI where each bus master has its own programming i/f -- so I didn't really dig this example. WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common
Hello. On 02/04/2013 08:02 PM, Felipe Balbi wrote: On Mon, Feb 04, 2013 at 08:36:38PM +0300, Sergei Shtylyov wrote: opted out of it. From the top of my head we have CPPI 3.x, CPPI 4.1, Inventra DMA, OMAP sDMA and ux500 DMA engines supported by the driver. Granted, CPPI 4.1 makes some assumptions about the fact that it's handling USB tranfers, What CPPI 4.1 code makes this assumptions? MUSB DMA driver? Then it's just HW makes the asumptions Not true at all. There is a separate set of registers (at offset 0) which copes with USB specifics, but CPPI 4.1 itself doesn't know about USB anything. CPPI 4.1 was made for USB transfers. Utter nonsense, CPPI 4.1 is hardware abstract DMA engine. It's used for Ethernet transfers on out-of-tree platforms like mach-avalanche/. It's just the way the driver was written that it used both sets of registers but this needs to be changed into more abstacted accesses to the USB-specific part, to cope with it being different on different platfroms, like AM35x. The driver as it was last posted, just needs rework now. are you saying you will do the work ? My efforts stopped to be funded by MV back in 2010. Hell, I'm not even working in MV as I used to, hanging on the verge of my current contract being terminated. Don't know, I was quite content with the abstraction when writing CPPI 4.1 driver for MUSB... look closer. The whole: if (is_cppi()) foo(); else if (is_inventra()) bar(); else if (is_omap_sdma()) baz(); is bogus. That part -- yes. There were attempt to get rid of this, but without changing the DMA API. It was left halfway done after my only critical comment, IIRC. Will we ever see the continuation of this effort? patches are welcome There was a patch, it only needed comments addressed. I think the author was Heikki Krogerus. As for me, my time is very limited, so be thankful even for those scarce patches I'm sending out now -- I'm doing them in my copious free time. WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common
Hello. On 02-02-2013 14:18, Russell King - ARM Linux wrote: On Fri, Feb 01, 2013 at 11:49:11PM +0300, Sergei Shtylyov wrote: good point, do you wanna send some patches ? I have already sent them countless times and even stuck CPPI 4.1 support (in arch/arm/common/cppi41.c) in Russell's patch system. TI requested to remove the patch. :-( sticking into arch/arm/common/ wasn't a nice move. But then again, so wasn't asking for the patch to be removed :-s Err, patches don't get removed, they get moved to 'discarded'. Any chance to bring it back to life? :-) Although... drivers/usb/musb/cppi41.c would need to be somewhat reworked for at least AM35x and I don't have time. But that may change, of course. Right, I've just looked back at the various meeting minutes from December 2010 when the CPPI stuff was discussed. Yes, I archive these things and all email discussions for referencing in cases like this. Thanks. Unfortunately, they do not contain any useful information other than the topic having been brought up. At that point, the CPPI stuff was in mach-davinci, and I had suggested moving it into drivers/dma. I don't remember that, probably was out of the loop again. I looked back at the history of CPPI 4.1 driver related threads, and found that Kevin Hilman gas suggested it too while the driver was in mach-davinci/ still... The result of that was to say that it doesn't fit the DMA engine APIs. Right, I tried to fit it (in my thought only though) in and it didn't work out. I remember this as a discussion happening post me sending the patch to the patch system and it being discarded... Well, actually before doing this too... So someone came up with the idea of putting it in arch/arm/common - which Probably was me. No, it was someone from TI. There was also idea of putting it into drivers/usb/musb/ -- which TI indeed followed in its Arago prject. I firmly denied that suggestion. Moving it to drivers/usb/ is probably the reason TI has been quite content with the situation -- their clients kept receiving MUSB DMA support on both OMAP-L1x and then Sitara, so all looked well for them. I frankly ignored by email (how long have we been saying no drivers in arch/arm ?) Well, maybe you should have said it one more time for those who were late in the game like me. But there *are* drivers there! And look at edma.c which is about to be moved there... Anyway, I haven't seen such warnings, probably was too late in the game. I've already objected about the header moving to some random place in arch/arm/include. Really, edma.c needs to find another home too - but there's a difference here. edma.c is already present under arch/arm. CPPI is _not_. CPPI is new code appearing under arch/arm (you can see that for yourself by looking at the diffstat of 6305/1... it doesn't move files, it adds new code.) Yes, of course, that's clear. Now, it would've been discussed in that meeting, but unfortunately no record exists of that. What does follow that meeting is a discussion trail. From what I can see there, but it looks to me like the decision was taken to move it to the DMA engine API, and work on sorting out MUSB was going to commence. The last email in that says I'll get to that soon... and that is also the final email I have on this topic. I guess if nothing has happened... Shrug, that's someone elses problem. Well, as usual... :-( Anyway, the answer for putting it in arch/arm/common hasn't changed, and really, where we are now, post Linus having a moan about the size of arch/arm, that answer is even more concrete in the negative. It's 54K of code which should not be under arch/arm at all. Anyway, if you need to look at the patch, it's 6305/1. Typing into the summary search box 'cppi' found it in one go. Thanks, I remember this variant was under arch/arm/common/. Now however, I see what happened to that variant in somewhat different light. Looks like it was entirely your decision to discard the patch, without TI's request... Firstly, it is *my* perogative to say no to anything in arch/arm, and I really don't have to give reasons for it if I choose to. That's clear. You're the ARM King. :-) Secondly, it *was* discussed with TI, and the following thread of discussion (threaded to the minutes email) shows that *something* was going to happen _as a result of that meeting_ to address the problem of it being under arch/arm. And *therefore* it was discarded from the patch system - because there was expectation that it was going to get fixed. For christ sake, someone even agreed to do it. Even a target was mentioned, of 2.6.39. That was mentioned on 7th December 2010. And 6305/1 was discarded on 8th December 2010. Cause and effect. And yes, *you* were not part of that discussion. You work for Montavista which contracts with TI to provide this support
Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common
Hello. On 02-02-2013 16:17, Russell King - ARM Linux wrote: good point, do you wanna send some patches ? I have already sent them countless times and even stuck CPPI 4.1 support (in arch/arm/common/cppi41.c) in Russell's patch system. TI requested to remove the patch. :-( sticking into arch/arm/common/ wasn't a nice move. But then again, so wasn't asking for the patch to be removed :-s Err, patches don't get removed, they get moved to 'discarded'. Any chance to bring it back to life? :-) Although... drivers/usb/musb/cppi41.c would need to be somewhat reworked for at least AM35x and I don't have time. But that may change, of course. Right, I've just looked back at the various meeting minutes from December 2010 when the CPPI stuff was discussed. Yes, I archive these things and all email discussions for referencing in cases like this. Thanks. Thanks again for taking your time to rummage thru the mail archives. I also did it, however not thru all threads (it turned out that the placement of CPPI 4.1 code was discussed also in the MUSB DMA driver related threads). Anyway, I was not a position to do extensive searching as it was already dead of the night in Moscow. Unfortunately, they do not contain any useful information other than the topic having been brought up. At that point, the CPPI stuff was in mach-davinci, and I had suggested moving it into drivers/dma. I don't remember that, probably was out of the loop again. Here you go - this goes back even _further_ - November 2009 - on the mailing list. The entire thread: http://lists.arm.linux.org.uk/lurker/thread/20091102.105759.a54cf3f5.en.html And selected emails from it: http://lists.arm.linux.org.uk/lurker/message/20091102.103706.38c029b5.en.html On Mon, Nov 02, 2009 at 10:37:06AM +, I wrote: | On Mon, Nov 02, 2009 at 04:27:59PM +0530, Gupta, Ajay Kumar wrote: | Another option is to create arch/arm/ti-common to place all TI platform's | common software, such as CPPI4.1 used both in DA8xx and AM3517. | | No thanks. I really don't see why we should allow TI to have yet more | directories scattered throughout the tree that are out of place with | existing conventions. | | And what is this CPPI thing anyway? | | http://acronyms.thefreedictionary.com/CPPI | | Communications Port Programming Interface seems to be about the best | applicable one from that list! | | If it's a USB DMA device (from the patches I can find, that seems to be | the case) then why can't it live in drivers/usb or drivers/dma ? And again: http://lists.arm.linux.org.uk/lurker/message/20091102.115458.61cde450.en.html On Mon, Nov 02, 2009 at 11:54:58AM +, I wrote: | On Mon, Nov 02, 2009 at 04:27:59PM +0530, Gupta, Ajay Kumar wrote: | CPPI4.1 DMA engine can be used either by USB or by Ethernet interface though | currently only USB is using it but in future even Ethernet devices may use it. | | drivers/dma does seem to be the right place for this. http://lists.arm.linux.org.uk/lurker/message/20091102.110217.adec3ca7.en.html Even Felipe Balbi said so: | you might want to provide support for it via drivers/dma and for the | musb stuff, you just add the wrappers musb uses. See how tusb6010_omap.c | uses OMAP's system dma which is also used by any other driver which | requests a dma channel. And it seems that _YOU_ did get the message - see your quoted text in: http://lists.arm.linux.org.uk/lurker/message/20091230.132240.ecd56b3d.en.html We're currently having it there but the matter is it should be shred between different platforms, so arch/arm/common/ seems like the right place (which Russell didn't like, suggesting ill suited for that drivers/dma/ instead). See - you acknowledge here that I don't like it. So you _KNOW_ my views on it in December 2009, contary to what you're saying in this thread. OK, now it seems I misremembered. Yet, you persisted with putting it in arch/arm/common: Being unable to fit it into drivers/dma/, and loating to place it into drivers/usb/, I had no other option. :-) http://lists.arm.linux.org.uk/lurker/message/20100515.181453.472c7c10.en.html | Changes since the previous version: | - moved everything from arch/arm/mach-davinci/ to arch/arm/common/; | - s/CONFIG_CPPI41/CONFIG_TI_CPPI41/, made that option invisible; | - added #include linux/slab.h for kzalloc(); | - switched alloc_queue() and cppi41_queue_free() to using bit operations; | - replaced 'static' linking_ram[] by local variable in cppi41_queue_mgr_init(); | - fixed pr_debug() in cppi41_dma_ctrlr_init() to print the real queue manager #. So, see, I had already objected to it being in arch/arm well before you stuck your patch into the patch system. And somehow you think that ignoring my previous comments and doing it anyway will result in progress? I probably did that out of hopelessness partly. So, let's recap. The timeline behind this is: + 2 Nov 2009: Question asked
Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common
Hello. On 02-02-2013 20:45, Russell King - ARM Linux wrote: There are two people on this thread CC list who were also involved or CC'd on the mails from the thread in 2010... Tony and Felipe. Unfortunately, the person who agreed to do the work is no longer in the land of the living. Yes I know it's inconvenient for people to die when they've still got lots of important work to do but that's what can happen... Hm... wasn't it David Brownell? He's the only person who I know has died recently who has dealt with DaVinci, MUSB and the releated stuff. Actually, it wasn't David who was going to do it - that's where the email thread gets messy because the mailer David was using makes no distinction in text format between what bits of text make up the original email being replied to, and which bits of text are written by David. Hm, strange... It might have been Felipe; there appears to be an email from Felipe saying that as the immediate parent to David's email. But that's not really the point here. The point is that _someone_ agreed to put the work in, and _that_ agreement is what caused the patch to be discarded. And, as I've already explained, you brought up the subject of it being discarded shortly after, and it got discussed there _again_, and the same things were said _again_ by at least two people about it being in drivers/dma. It wasn't said that somebody concrete was going to work on it. I had to explcitly write an email laying all further responsibility on CPPI 4.1 support on TI back then. But anyway, that's all past history. What was said back then about it being elsewhere in the tree is just as relevant today as it was back then. The only difference is that because that message wasn't received, it's now two years later with no progress on that. And that's got *nothing* what so ever to do with me. Yes, of course. In my original mail that started the discussion I said that we have to wait indefinitely for TI to write the new DMA driver. I just wondered wouldn't it be better to use the same approach as for EDMA with transitioning to drivers/dma/ step by step. I know people like to blame me just because I'm apparantly the focus of the blame culture, but really this is getting beyond a joke. So, I want an apology from you for your insistance that I'm to blame for this. OK, I apologise if you consider yourself the target of my blame. My aim was rather to establish the truth about that decision taken back in Dec 2010 -- which we seem to have achieved. Moreover, _I_ want to know what is going to happen in the future with this so that I don't end up being blamed anymore for the lack of progress on this issue. Nothing. My blame for the lack of progress has long been laid on TI, after I explictly passed the responsibility for the driver to them. My intent with the mail that started the discussion was to probe whether we still have another opportunity of having MUSB DMA support for OMAP-L1x and Sitara. I just thought that you might have changed your mind somehow on the matter. WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common
Hello. On 02-02-2013 22:07, Matt Porter wrote: Move mach-davinci/dma.c to common/edma.c so it can be used by OMAP (specifically AM33xx) as well. I think this should rather go to drivers/dma/? No, this is the private EDMA API. It's the analogous thing to the private OMAP dma API that is in plat-omap/dma.c. The actual dmaengine driver is in drivers/dma/edma.c as a wrapper around this...same way OMAP DMA engine conversion is being done. Keeps me wondering why we couldn't have the same with CPPI 4.1 when I proposed that, instead of waiting indefinitely for TI to convert it to drivers/dma/ directly. We could have working MUSB DMA on OMAP-L1x/Sitara all this time... Sigh. That is a shame. Yeah, I've pointed out that I was doing this exactly the same way as was acceptable for the OMAP DMA conversion since it was in RFC. The reasons are sound since in both cases, we have many drivers to convert that need to continue using the private DMA APIs. In case of CPPI 4.1, we'd only have to convert MUSB DMA driver. Other in-tree CPPI 4.1 having SoCs don't use it for anything but MUSB -- it even is sub-block of their MUSB device, AFAIK (I maybe wrong about Sitaras -- I don't know them well). Well, it's pretty clear to me now that there's good reason for it not landing in arch/arm/ so the obvious path is to do the dmaengine conversion and put it in drivers/dma/ if it's really a generic dma engine. I'm not sure why you express concern over the dma engine api not fitting with CPPI 4.1? It's not a DMA controller only, it's 3 distinct devices, with the DMA controller being one among them and using another one, the queue manager, as some sort of proxy. The third device doesn't exist on OMAP-L1x SoCs -- it's the buffer manager. If it doesn't work, work with Vinod to fix the api. It's expected, I'm working on dmaengine API changes right now to deal with a limitation of EDMA that needs to be abstracted. Sorry, now it's TI's task. I no longer have time to work on this (my internal project to push OMAP-L1x support upstream has expired at Sep 2010) and my future in MV is very uncertain at this moment. Most probably I'll leave it (or be forced to leave). As pointed out, edma.c is already here in arch/arm/, so moving it doesn't add something new. It does let us regression test all platforms that use it (both Davinci and AM33xx) as I go through the conversion process. Could have been the same with CPPI 4.1 in theory if it was added to mach-davinci/ back in 2009... we'd then only have to move it. EDMA code is much older of course, so it's probably more justified. -Matt WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common
Hello. On 02-02-2013 23:55, Matt Porter wrote: Move mach-davinci/dma.c to common/edma.c so it can be used by OMAP (specifically AM33xx) as well. I think this should rather go to drivers/dma/? No, this is the private EDMA API. It's the analogous thing to the private OMAP dma API that is in plat-omap/dma.c. The actual dmaengine driver is in drivers/dma/edma.c as a wrapper around this...same way OMAP DMA engine conversion is being done. Keeps me wondering why we couldn't have the same with CPPI 4.1 when I proposed that, instead of waiting indefinitely for TI to convert it to drivers/dma/ directly. We could have working MUSB DMA on OMAP-L1x/Sitara all this time... Sigh. That is a shame. Yeah, I've pointed out that I was doing this exactly the same way as was acceptable for the OMAP DMA conversion since it was in RFC. The reasons are sound since in both cases, we have many drivers to convert that need to continue using the private DMA APIs. In case of CPPI 4.1, we'd only have to convert MUSB DMA driver. Other in-tree CPPI 4.1 having SoCs don't use it for anything but MUSB -- it even is sub-block of their MUSB device, AFAIK (I maybe wrong about Sitaras -- I don't know them well). Well, it's pretty clear to me now that there's good reason for it not landing in arch/arm/ so the obvious path is to do the dmaengine conversion and put it in drivers/dma/ if it's really a generic dma engine. I'm not sure why you express concern over the dma engine api not fitting with CPPI 4.1? It's not a DMA controller only, it's 3 distinct devices, with the DMA controller being one among them and using another one, the queue manager, as some sort of proxy. The third device doesn't exist on OMAP-L1x SoCs -- it's the buffer manager. Interesting, and you don't see a way to have this split between dmaengine and the musb driver? This can't be split into the MUSB DMA driver, as the neither of CPPI 4.1 devices are really MUSB specific. The support should be generic. This all assumes there's even a match as RMK did raise concerns elsewhere in this thread. Didn't quite get this sentence. If it doesn't work, work with Vinod to fix the api. It's expected, I'm working on dmaengine API changes right now to deal with a limitation of EDMA that needs to be abstracted. Sorry, now it's TI's task. I no longer have time to work on this (my internal project to push OMAP-L1x support upstream has expired at Sep 2010) If not somewhat earlier... anyway, I wasn't able to spent much time on this project in 2010. and my future in MV is very uncertain at this moment. Most probably I'll leave it (or be forced to leave). Too bad, it's certainly somebody's task. The people employed by TI have names too. ;) I suspect it falls to Felipe or Kishon these days, but I try to avoid USB as it's generally a source of pain. I'm probably masochistic then since I'm still sending MUSB patches, eben though I wasn't working on it at MV until I switched to Kontron boards 2 weeks ago. Now my task is fixing USB bugs on real Sitara with dual MUSB. :-) As pointed out, edma.c is already here in arch/arm/, so moving it doesn't add something new. It does let us regression test all platforms that use it (both Davinci and AM33xx) as I go through the conversion process. Could have been the same with CPPI 4.1 in theory if it was added to mach-davinci/ back in 2009... we'd then only have to move it. EDMA code is I don't know why Kevin didn't merge it then. I remembere he didn't like that it was not a proper platform driver and was tied with the platform code thru some variables, and I refused to change that... much older of course, so it's probably more justified. Absolutely, timing is everything. I can assure you I've flamed enough people internally about leaving this EDMA dmaengine conversion for so long. As you might have guessed, AM33xx is a bit of a driver for it, but all of this would have been quite a bit simpler had somebody taken a little effort and moved edma to dmaengine years ago when slave dma support was added to dmaengine. ;) Hm, it seems to have happened back in 2008 when I was working on CPPI 4.1 code. Too bad I only knew that drivers/dma/ are for accelerating RAIDs back then (if actually noot later than that). TI seems to put more efforts into its Arago project than in supoporting the cutting edge (or not) CPUs in mainline -- at lest things seem go there first. Many Arago patches never reached mainline (not that they can be applied without cleanup though). -Matt WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common
Hello. On 02/01/2013 09:49 PM, Matt Porter wrote: Move mach-davinci/dma.c to common/edma.c so it can be used by OMAP (specifically AM33xx) as well. I think this should rather go to drivers/dma/? No, this is the private EDMA API. It's the analogous thing to the private OMAP dma API that is in plat-omap/dma.c. The actual dmaengine driver is in drivers/dma/edma.c as a wrapper around this...same way OMAP DMA engine conversion is being done. Keeps me wondering why we couldn't have the same with CPPI 4.1 when I proposed that, instead of waiting indefinitely for TI to convert it to drivers/dma/ directly. We could have working MUSB DMA on OMAP-L1x/Sitara all this time... Sigh. -Matt WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common
Hello. On 02/01/2013 09:58 PM, Felipe Balbi wrote: Move mach-davinci/dma.c to common/edma.c so it can be used by OMAP (specifically AM33xx) as well. I think this should rather go to drivers/dma/? No, this is the private EDMA API. It's the analogous thing to the private OMAP dma API that is in plat-omap/dma.c. The actual dmaengine driver is in drivers/dma/edma.c as a wrapper around this...same way OMAP DMA engine conversion is being done. Keeps me wondering why we couldn't have the same with CPPI 4.1 when I proposed that, instead of waiting indefinitely for TI to convert it to drivers/dma/ directly. We could have working MUSB DMA on OMAP-L1x/Sitara all this time... Sigh. good point, do you wanna send some patches ? I have already sent them countless times and even stuck CPPI 4.1 support (in arch/arm/common/cppi41.c) in Russell's patch system. TI requested to remove the patch. :-( I guess to make the MUSB side simpler we would need musb-dma-engine glue to map dmaengine to the private MUSB API. Then we would have some starting point to also move inventra (and anybody else) to dmaengine API. Why? Inventra is a dedicated device's private DMA controller, why make universal DMA driver for it? Once that's done, we drop MUSB's private API. Don't think it's a good idea. WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common
Hello. On 02-02-2013 0:56, Felipe Balbi wrote: good point, do you wanna send some patches ? I have already sent them countless times and even stuck CPPI 4.1 support (in arch/arm/common/cppi41.c) in Russell's patch system. TI requested to remove the patch. :-( sticking into arch/arm/common/ wasn't a nice move. Like with EDMA we have nothing else to do with CPPI 4.1 being shared by DaVinci-like and OMAP-like SOCs. Thank TI for creatring this mess. And actually even that is not a good place since I think I know of a MIPS SoC having CPPI 4.1 as well, just out of tree. But then again, so wasn't asking for the patch to be removed :-s Unfortunately, Russell has done it -- all that was discuseed without me in the loop even. :-/ I guess to make the MUSB side simpler we would need musb-dma-engine glue to map dmaengine to the private MUSB API. Then we would have some starting point to also move inventra (and anybody else) to dmaengine API. Why? Inventra is a dedicated device's private DMA controller, why make universal DMA driver for it? because it doesn't make sense to support multiple DMA APIs. We can check from MUSB's registers if it was configured with Inventra DMA support and based on that we can register MUSB's own DMA Engine to dmaengine API. I still disagree. IMO drivers/dma/ is for standalone DMA engines. Else we could stick every bus mastering device's DMA engines there. CPPI 4.1 is in design standlone DMA engine, despite all in-tree implementations having it as subblock of MUSB and serving only MUSB. WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common
Hello. On 01-02-2013 22:59, Matt Porter wrote: Move mach-davinci/dma.c to common/edma.c so it can be used by OMAP (specifically AM33xx) as well. I think this should rather go to drivers/dma/? No, this is the private EDMA API. It's the analogous thing to the private OMAP dma API that is in plat-omap/dma.c. The actual dmaengine driver is in drivers/dma/edma.c as a wrapper around this...same way OMAP DMA engine conversion is being done. Keeps me wondering why we couldn't have the same with CPPI 4.1 when I proposed that, instead of waiting indefinitely for TI to convert it to drivers/dma/ directly. We could have working MUSB DMA on OMAP-L1x/Sitara all this time... Sigh. That is a shame. Yeah, I've pointed out that I was doing this exactly the same way as was acceptable for the OMAP DMA conversion since it was in RFC. The reasons are sound since in both cases, we have many drivers to convert that need to continue using the private DMA APIs. In case of CPPI 4.1, we'd only have to convert MUSB DMA driver. Other in-tree CPPI 4.1 having SoCs don't use it for anything but MUSB -- it even is sub-block of their MUSB device, AFAIK (I maybe wrong about Sitaras -- I don't know them well). -Matt WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common
Hello. On 02-02-2013 1:30, Russell King - ARM Linux wrote: On Fri, Feb 01, 2013 at 11:49:11PM +0300, Sergei Shtylyov wrote: good point, do you wanna send some patches ? I have already sent them countless times and even stuck CPPI 4.1 support (in arch/arm/common/cppi41.c) in Russell's patch system. TI requested to remove the patch. :-( sticking into arch/arm/common/ wasn't a nice move. But then again, so wasn't asking for the patch to be removed :-s Err, patches don't get removed, they get moved to 'discarded'. Any chance to bring it back to life? :-) Although... drivers/usb/musb/cppi41.c would need to be somewhat reworked for at least AM35x and I don't have time. But that may change, of course. I guess to make the MUSB side simpler we would need musb-dma-engine glue to map dmaengine to the private MUSB API. Then we would have some starting point to also move inventra (and anybody else) to dmaengine API. Why? Inventra is a dedicated device's private DMA controller, why make universal DMA driver for it? because it doesn't make sense to support multiple DMA APIs. We can check from MUSB's registers if it was configured with Inventra DMA support and based on that we can register MUSB's own DMA Engine to dmaengine API. Hang on. This is one of the DMA implementations which is closely coupled with the USB and only the USB? If it is... I thought this had been discussed _extensively_ before. I thought the resolution on it was: 1. It would not use the DMA engine API. 2. It would not live in arch/arm. 3. It would be placed nearby the USB driver it's associated with. (1) because we don't use APIs just for the hell of it - think. Do we use the DMA engine API for PCI bus mastering ethernet controllers? No. Do we use it for PCI bus mastering SCSI controllers? No. Because the DMA is integral to the rest of the device. The DMA engine API only makes sense if the DMA engine is a shared system resource. Thanks for such extensive wording in the support of my point. :-) WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common
Hello. On 02-02-2013 1:30, Russell King - ARM Linux wrote: On Fri, Feb 01, 2013 at 11:49:11PM +0300, Sergei Shtylyov wrote: good point, do you wanna send some patches ? I have already sent them countless times and even stuck CPPI 4.1 support (in arch/arm/common/cppi41.c) in Russell's patch system. TI requested to remove the patch. :-( sticking into arch/arm/common/ wasn't a nice move. But then again, so wasn't asking for the patch to be removed :-s Err, patches don't get removed, they get moved to 'discarded'. I guess to make the MUSB side simpler we would need musb-dma-engine glue to map dmaengine to the private MUSB API. Then we would have some starting point to also move inventra (and anybody else) to dmaengine API. Why? Inventra is a dedicated device's private DMA controller, why make universal DMA driver for it? because it doesn't make sense to support multiple DMA APIs. We can check from MUSB's registers if it was configured with Inventra DMA support and based on that we can register MUSB's own DMA Engine to dmaengine API. Hang on. This is one of the DMA implementations which is closely coupled with the USB and only the USB? If it is... I thought this had been discussed _extensively_ before. I thought the resolution on it was: 1. It would not use the DMA engine API. 2. It would not live in arch/arm. 3. It would be placed nearby the USB driver it's associated with. Note that all this doesn't apply to CPPI 4.1 controller (as contrasted to CPPI 3.0 support in MUSB aand EMAC drivers) -- it's shared by design. Just the implementations that are in tree have it as MUSB's sub-block, serving only MUSB. WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common
Hello. On 02-02-2013 4:44, Russell King - ARM Linux wrote: On Fri, Feb 01, 2013 at 11:49:11PM +0300, Sergei Shtylyov wrote: good point, do you wanna send some patches ? I have already sent them countless times and even stuck CPPI 4.1 support (in arch/arm/common/cppi41.c) in Russell's patch system. TI requested to remove the patch. :-( sticking into arch/arm/common/ wasn't a nice move. But then again, so wasn't asking for the patch to be removed :-s Err, patches don't get removed, they get moved to 'discarded'. Any chance to bring it back to life? :-) Although... drivers/usb/musb/cppi41.c would need to be somewhat reworked for at least AM35x and I don't have time. But that may change, of course. Right, I've just looked back at the various meeting minutes from December 2010 when the CPPI stuff was discussed. Yes, I archive these things and all email discussions for referencing in cases like this. Thanks. Unfortunately, they do not contain any useful information other than the topic having been brought up. At that point, the CPPI stuff was in mach-davinci, and I had suggested moving it into drivers/dma. I don't remember that, probably was out of the loop again. The result of that was to say that it doesn't fit the DMA engine APIs. I remember this as a discussion happening post me sending the patch to the patch system and it being discarded... So someone came up with the idea of putting it in arch/arm/common - which Probably was me. There was also idea of putting it into drivers/usb/musb/ -- which TI indeed followed in its Arago prject. I firmly denied that suggestion. I frankly ignored by email (how long have we been saying no drivers in arch/arm ?) But there *are* drivers there! And look at edma.c which is about to be moved there... Anyway, I haven't seen such warnings, probably was too late in the game. Now, it would've been discussed in that meeting, but unfortunately no record exists of that. What does follow that meeting is a discussion trail. From what I can see there, but it looks to me like the decision was taken to move it to the DMA engine API, and work on sorting out MUSB was going to commence. The last email in that says I'll get to that soon... and that is also the final email I have on this topic. I guess if nothing has happened... Shrug, that's someone elses problem. Well, as usual... :-( Anyway, the answer for putting it in arch/arm/common hasn't changed, and really, where we are now, post Linus having a moan about the size of arch/arm, that answer is even more concrete in the negative. It's 54K of code which should not be under arch/arm at all. Anyway, if you need to look at the patch, it's 6305/1. Typing into the summary search box 'cppi' found it in one go. Thanks, I remember this variant was under arch/arm/common/. Now however, I see what happened to that variant in somewhat different light. Looks like it was entirely your decision to discard the patch, without TI's request... WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v6 1/2] ARM: davinci: Remoteproc driver support for OMAP-L138 DSP
Hello.
On 26-01-2013 6:45, Robert Tivy wrote:
Adding a remoteproc driver implementation for OMAP-L138 DSP
You forgot to sign off the patch.
diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 96ce101..e923599 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
[...]
@@ -41,4 +41,28 @@ config STE_MODEM_RPROC
This can be either built-in or a loadable module.
If unsure say N.
+config DA8XX_REMOTEPROC
+ tristate DaVinci DA850/OMAPL138 remoteproc support (EXPERIMENTAL)
Neither DA850 nor OMAP-L138 are true DaVinci processors. Please drop the
DaVinci word.
+ depends on ARCH_DAVINCI_DA850
It's also not clear why you limit the driver d\to DA850 while you call it
da8xx_remoteproc.c. There's at least one more member to DA8xx familiy (at
least supported in the community): DA830/OMAP-L137.
+ select REMOTEPROC
+ select RPMSG
+ select CMA
+ default n
+ help
+ Say y here to support DaVinci DA850/OMAPL138 remote processors
Same here.
diff --git a/drivers/remoteproc/da8xx_remoteproc.c
b/drivers/remoteproc/da8xx_remoteproc.c
new file mode 100644
index 000..c6eb6bf
--- /dev/null
+++ b/drivers/remoteproc/da8xx_remoteproc.c
@@ -0,0 +1,327 @@
+/*
+ * Remote processor machine-specific module for DA8XX
+ *
+ * Copyright (C) 2013 Texas Instruments, Inc.
+ *
+ * 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.
+ */
+
+#include linux/bitops.h
+#include linux/clk.h
+#include linux/err.h
+#include linux/interrupt.h
+#include linux/io.h
+#include linux/irq.h
+#include linux/kernel.h
+#include linux/module.h
+#include linux/platform_device.h
+#include linux/remoteproc.h
+
+#include mach/clock.h /* for davinci_clk_reset_assert/deassert() */
+
+#include remoteproc_internal.h
+
+static char *da8xx_fw_name;
+module_param(da8xx_fw_name, charp, S_IRUGO);
+MODULE_PARM_DESC(da8xx_fw_name,
+ \n\t\tName of DSP firmware file in /lib/firmware);
+
+/*
+ * OMAP-L138 Technical References:
+ * http://www.ti.com/product/omap-l138
+ */
+#define SYSCFG_CHIPSIG_OFFSET 0x174
+#define SYSCFG_CHIPSIG_CLR_OFFSET 0x178
mach/da8xx.h has SYSCFG register #define's ending with '_REG', not
'_OFFSET' -- I'd like this tradition to be kept. And perhaps we should #define
these registers there instead of the driver?
+#define SYSCFG_CHIPINT0 BIT(0)
+#define SYSCFG_CHIPINT1 BIT(1)
+#define SYSCFG_CHIPINT2 BIT(2)
+#define SYSCFG_CHIPINT3 BIT(3)
DA830/OMAP-l137 has the same registers. Only the datasheet calls the bits
CHIPSIGn there. Bit 4 also exists and means DSP NMI.
+static int da8xx_rproc_probe(struct platform_device *pdev)
[...]
+ chipsig = devm_request_and_ioremap(dev, chipsig_res);
+ if (!chipsig) {
+ dev_err(dev, unable to map CHIPSIG register\n);
+ return -EINVAL;
Why -EINVAL? Comment to devm_request_and_ioremap() suggests returning
-EADDRNOTAVAIL.
+ }
+
+ bootreg = devm_request_and_ioremap(dev, bootreg_res);
+ if (!bootreg) {
+ dev_err(dev, unable to map boot register\n);
+ return -EINVAL;
Same here.
+static int __devexit da8xx_rproc_remove(struct platform_device *pdev)
+{
+ struct rproc *rproc = platform_get_drvdata(pdev);
+ struct da8xx_rproc *drproc = (struct da8xx_rproc *)rproc-priv;
+ int ret;
+
+ /*
+* The devm subsystem might end up releasing things before
+* freeing the irq, thus allowing an interrupt to sneak in while
+* the device is being removed. This should prevent that.
+*/
+ disable_irq(drproc-irq);
Will the IRQ be enabled properly upon reloading the driver? You're
effectively disabling it twice, once here and once in devm_free_irq(), aren't you?
+static struct platform_driver da8xx_rproc_driver = {
+ .probe = da8xx_rproc_probe,
+ .remove = __devexit_p(da8xx_rproc_remove),
Isn't _devexit_p() removed now? I thought __devinit and friends have all
been removed in 3.8-rc1...
diff --git a/drivers/remoteproc/remoteproc_core.c
b/drivers/remoteproc/remoteproc_core.c
index dd3bfaf..ac4449a 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -1222,19 +1222,39 @@ struct rproc *rproc_alloc(struct device *dev, const
char *name,
const char *firmware, int len)
{
struct rproc *rproc;
+ char *template = rproc-%s-fw;
+ char *p;
if (!dev || !name || !ops)
return NULL;
+if (!firmware)
It makes sense to use {} despite singkle-statement branch.
+/*
Indent with tabs please.
+* Make room for default firmware name (minus %s plus '\0').
+* If the caller didn't pass in a
Re: [PATCH v6 2/2] ARM: davinci: Remoteproc platform device creation data/code
Hello.
On 26-01-2013 6:45, Robert Tivy wrote:
Added a new remoteproc platform device for DA8XX. Contains CMA-based
reservation of physical memory block. A new kernel command-line
parameter has been added to allow boot-time specification of the
physical memory block.
No signoff again.
diff --git a/arch/arm/mach-davinci/devices-da8xx.c
b/arch/arm/mach-davinci/devices-da8xx.c
index fb2f51b..a455e5c 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c
[...]
@@ -706,6 +706,96 @@ int __init da850_register_mmcsd1(struct davinci_mmc_config
*config)
}
#endif
+static struct resource da8xx_rproc_resources[] = {
+ { /* DSP boot address */
+ .start = DA8XX_SYSCFG0_BASE + DA8XX_HOST1CFG_REG,
+ .end= DA8XX_SYSCFG0_BASE + DA8XX_HOST1CFG_REG + 3,
+ .flags = IORESOURCE_MEM,
+ },
+ { /* DSP interrupt registers */
+ .start = DA8XX_SYSCFG0_BASE + DA8XX_CHIPSIG_REG,
+ .end= DA8XX_SYSCFG0_BASE + DA8XX_CHIPSIG_REG + 7,
+ .flags = IORESOURCE_MEM,
Does it really make sense to pass these as 2 resources -- they have the
same base address?
+int __init da8xx_register_rproc(void)
+{
+ int ret;
+
+ ret = platform_device_register(da8xx_dsp);
+ if (ret) {
+ pr_err(%s: platform_device_register: %d\n, __func__, ret);
Better message would be can't register DSP device.
+
Empty line hardly needed here.
+ return ret;
Not needed here, just move it outside the {} to replace 'return 0'.
+ }
+
+ return 0;
+};
+
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v6 1/2] ARM: davinci: Remoteproc driver support for OMAP-L138 DSP
Hello. On 26-01-2013 6:45, Robert Tivy wrote: Adding a remoteproc driver implementation for OMAP-L138 DSP diff --git a/drivers/remoteproc/da8xx_remoteproc.c b/drivers/remoteproc/da8xx_remoteproc.c new file mode 100644 index 000..c6eb6bf --- /dev/null +++ b/drivers/remoteproc/da8xx_remoteproc.c @@ -0,0 +1,327 @@ [...] +#define SYSCFG_CHIPSIG_OFFSET 0x174 +#define SYSCFG_CHIPSIG_CLR_OFFSET 0x178 Wait, you don't even use these #define's -- why they're here at all? WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2] davinci_nand: fix modular build with CONFIG_OF=y
Hello. On 01/03/2013 09:27 PM, Sergei Shtylyov wrote: Commit cdeadd712f52b16a9285386d61ee26fd14eb4085 (mtd: nand: davinci: add OF support for davinci nand controller) has never been really build tested with the driver as a module. When the driver is built-in, the missing semicolon after structure initializer is compensated by MODULE_DEVICE_TABLE() macro being empty and so the initializer using the trailing semicolon on the next line; when the driver is built as a module, compilation error ensues, and as the 'davinci_all_defconfig' has the NAND driver modular, this error prevents DaVinci family kernel from building... Signed-off-by: Sergei Shtylyov sshtyl...@ru.mvista.com Cc: sta...@vger.kernel.org # 3.7 --- The patch is atop of the recent Linus' tree. OK, a week has passed. Anybody going to queue this up for 3.8? WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: Are TI DM8168/DM8148 considered DaVinci processors ?
Hello. On 03-01-2013 14:45, Kiril Maler wrote: best wishes for new 2013 :-) My question: are TI DM8168/DM8148 considered DaVinci processors ? 1. According to this page: yes http://e2e.ti.com/support/dsp/davinci_digital_media_processors/default.aspx 2. According to these pages: no http://processors.wiki.ti.com/index.php/DaVinci_PSP_Releases http://processors.wiki.ti.com/index.php/DaVinci_GIT_Linux_Kernel The DM8168 EVM board uses an obsolete 2.6.37-2.6.39 source http://processors.wiki.ti.com/index.php/DM816x_AM389x_PSP_User_Guide AFAIK, DM814x is not completely ported to mainline kernel. Take a look at below Link for latest DM814x releases http://processors.wiki.ti.com/index.php/Category:DM814x Thanks, but this was not my question. Investigating further, your link above leads to ... http://processors.wiki.ti.com/index.php/Category:EZSDK_Software_BOM there is link to linux git repository git://arago-project.org/git/projects/linux-omap3.git http://arago-project.org/git/projects/?p=linux-omap3.git;a=summary claiming: descripion: Linux PSP Integration/Staging Tree for ARM Cortex Based SoCs owner: Sriram Govindarajan last change: Wed, 12 Dec 2012 05:44:04 + and the linux tag is 04.04.00.01 for DM8168/8148 according to page http://processors.wiki.ti.com/index.php/EZSDK_Feature_List Going further: http://arago-project.org/wiki/index.php/Main_Page ... Arago Project is an open integration, build, and test infrastructure that provides a portal into how Texas Instruments creates customer ready Linux SDKs. Arago Project is an overlay for OpenEmbedded/Angstrom, which targets TI platforms OMAP3 (EVM and BeagleBoard) and DaVinci (6446, 355, 365, 6467...) and provides a verified, tested and supported subset of packages, built with a free and open toolchain (CodeSourcery Lite for now). IRC channel: #arago ... The arago git repository is frozen to 2.6.37. So I see as option extracting some DM81xx parts from arch/arm/mac-omap2 of Arago git repository, and posting them to this mailing list. We don't need OMAP patches here, there's linux-omap list for that. WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v6] ARM: mtd: nand: davinci: add OF support for davinci nand controller
Hello.
On 03-01-2013 11:03, Heiko Schocher wrote:
add OF support for the davinci nand controller.
Signed-off-by: Heiko Schocherh...@denx.de
Cc: davinci-linux-open-source@linux.davincidsp.com
Cc: linux-arm-ker...@lists.infradead.org
Cc: devicetree-disc...@lists.ozlabs.org
Cc: linux-...@lists.infradead.org
Cc: David Woodhousedw...@infradead.org
Cc: Grant Likelygrant.lik...@secretlab.ca
Cc: Sekhar Norinsek...@ti.com
Cc: Wolfgang Denkw...@denx.de
Cc: Scott Woodscottw...@freescale.com
[...]
diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
index d94b03c..f386b3c 100644
--- a/drivers/mtd/nand/davinci_nand.c
+++ b/drivers/mtd/nand/davinci_nand.c
[...]
@@ -518,9 +519,75 @@ static struct nand_ecclayout hwecc4_2048 __initconst = {
},
};
+#if defined(CONFIG_OF)
+static const struct of_device_id davinci_nand_of_match[] = {
+{.compatible = ti,davinci-nand, },
+{},
+}
I have only one question: have you ever try to compile this patch with
CONFIG_OF enabled? If you have, you would have noticed:
drivers/mtd/nand/davinci_nand.c:527: error: expected ‘,’ or ‘;’ before ‘extern’
+MODULE_DEVICE_TABLE(of, davinci_nand_of_match);
Hmm.. maybe this crept in later after I sent the patches? They were
pending for a while ... I compiled it just yet again (based on my
tree when I posted this patch based on commit:
I've just checked the archives: every patch version you posted had ';'
after '}' missing.
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v6] ARM: mtd: nand: davinci: add OF support for davinci nand controller
Hello.
On 03-01-2013 15:32, David Woodhouse wrote:
+#if defined(CONFIG_OF)
+static const struct of_device_id davinci_nand_of_match[] = {
+{.compatible = ti,davinci-nand, },
+{},
+}
+MODULE_DEVICE_TABLE(of, davinci_nand_of_match);
Hmm.. maybe this crept in later after I sent the patches? They were
pending for a while ... I compiled it just yet again (based on my
tree when I posted this patch based on commit:
I've just checked the archives: every patch version you posted had ';'
after '}' missing.
If it was built-in, rather than as a module, then MODULE_DEVICE_TABLE(…)
expands to nothing, and the structure gets to use the semicolon from the
end of that line.
Ah, that explains it: 'davinci_all_defconfig' has
CONFIG_MTD_NAND_DAVINCI=m, so that's how the error got triggered at last.
Probably worth adding this explanation to the changelog, how do you think?
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v6] ARM: mtd: nand: davinci: add OF support for davinci nand controller
Hello.
On 01/03/2013 02:39 PM, Sergei Shtylyov wrote:
+#if defined(CONFIG_OF)
+static const struct of_device_id davinci_nand_of_match[] = {
+{.compatible = ti,davinci-nand, },
+{},
+}
+MODULE_DEVICE_TABLE(of, davinci_nand_of_match);
Hmm.. maybe this crept in later after I sent the patches? They were
pending for a while ... I compiled it just yet again (based on my
tree when I posted this patch based on commit:
I've just checked the archives: every patch version you posted had ';'
after '}' missing.
If it was built-in, rather than as a module, then MODULE_DEVICE_TABLE(…)
expands to nothing, and the structure gets to use the semicolon from the
end of that line.
Ah, that explains it: 'davinci_all_defconfig' has
CONFIG_MTD_NAND_DAVINCI=m,
so that's how the error got triggered at last. Probably worth adding this
explanation to the changelog, how do you think?
OK, I take your silence as a sign of consent. Changelog indeed needs to be
rewritten a bit now.
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH v2] davinci_nand: fix modular build with CONFIG_OF=y
Commit cdeadd712f52b16a9285386d61ee26fd14eb4085 (mtd: nand: davinci: add OF
support for davinci nand controller) has never been really build tested with
the driver as a module. When the driver is built-in, the missing semicolon
after structure initializer is compensated by MODULE_DEVICE_TABLE() macro
being empty and so the initializer using the trailing semicolon on the next
line; when the driver is built as a module, compilation error ensues, and as
the 'davinci_all_defconfig' has the NAND driver modular, this error prevents
DaVinci family kernel from building...
Signed-off-by: Sergei Shtylyov sshtyl...@ru.mvista.com
Cc: sta...@vger.kernel.org # 3.7
---
The patch is atop of the recent Linus' tree.
Sekhar, have you build tested at least 3.8-rc1?
drivers/mtd/nand/davinci_nand.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux/drivers/mtd/nand/davinci_nand.c
===
--- linux.orig/drivers/mtd/nand/davinci_nand.c
+++ linux/drivers/mtd/nand/davinci_nand.c
@@ -523,7 +523,7 @@ static struct nand_ecclayout hwecc4_2048
static const struct of_device_id davinci_nand_of_match[] = {
{.compatible = ti,davinci-nand, },
{},
-}
+};
MODULE_DEVICE_TABLE(of, davinci_nand_of_match);
static struct davinci_nand_pdata
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v6] ARM: mtd: nand: davinci: add OF support for davinci nand controller
Hello.
On 07/30/2012 11:22 AM, Heiko Schocher wrote:
add OF support for the davinci nand controller.
Signed-off-by: Heiko Schocher h...@denx.de
Cc: davinci-linux-open-source@linux.davincidsp.com
Cc: linux-arm-ker...@lists.infradead.org
Cc: devicetree-disc...@lists.ozlabs.org
Cc: linux-...@lists.infradead.org
Cc: David Woodhouse dw...@infradead.org
Cc: Grant Likely grant.lik...@secretlab.ca
Cc: Sekhar Nori nsek...@ti.com
Cc: Wolfgang Denk w...@denx.de
Cc: Scott Wood scottw...@freescale.com
[...]
diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
index d94b03c..f386b3c 100644
--- a/drivers/mtd/nand/davinci_nand.c
+++ b/drivers/mtd/nand/davinci_nand.c
[...]
@@ -518,9 +519,75 @@ static struct nand_ecclayout hwecc4_2048 __initconst = {
},
};
+#if defined(CONFIG_OF)
+static const struct of_device_id davinci_nand_of_match[] = {
+ {.compatible = ti,davinci-nand, },
+ {},
+}
I have only one question: have you ever try to compile this patch with
CONFIG_OF enabled? If you have, you would have noticed:
drivers/mtd/nand/davinci_nand.c:527: error: expected ‘,’ or ‘;’ before ‘extern’
+MODULE_DEVICE_TABLE(of, davinci_nand_of_match);
No need to worry now, I'll send out the trivial patch...
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
[PATCH] davinci_nand: fix compilation with CONFIG_OF=y
Commit cdeadd712f52b16a9285386d61ee26fd14eb4085 (mtd: nand: davinci: add OF
support for davinci nand controller) obviously has never been really build
tested with CONFIG_OF=y. Now it prevents DaVinci family kernels from building:
all due to stupidly missing semicolon after a structure initializer...
Signed-off-by: Sergei Shtylyov sshtyl...@ru.mvista.com
Cc: sta...@vger.kernel.org # 3.7
---
Sekhar, have you build tested at least 3.8-rc1?
drivers/mtd/nand/davinci_nand.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux/drivers/mtd/nand/davinci_nand.c
===
--- linux.orig/drivers/mtd/nand/davinci_nand.c
+++ linux/drivers/mtd/nand/davinci_nand.c
@@ -523,7 +523,7 @@ static struct nand_ecclayout hwecc4_2048
static const struct of_device_id davinci_nand_of_match[] = {
{.compatible = ti,davinci-nand, },
{},
-}
+};
MODULE_DEVICE_TABLE(of, davinci_nand_of_match);
static struct davinci_nand_pdata
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v3 1/6] ARM: davinci: Changed pr_warning() to pr_warn() (part 1)
Hello.
On 14-11-2012 4:33, Robert Tivy wrote:
These subjects are not very good too -- it's better to specify the scope
of the changes, like ARM: DaVinci: DA850 EVM: change pr_warning() to pr_warn().
Also, while modifying those pr_warning() calls I changed hardcoded
function names to use '%s:, __func__' instead
Signed-off-by: Robert Tivy rt...@ti.com
---
Clean up files that will be otherwise modified in subsequent patch.
Applies to v3.7-rc2 tag (commit 6f0c0580b70c89094b3422ba81118c7b959c7556) of
Linus' mainline kernel at git.kernel.org.
arch/arm/mach-davinci/board-da850-evm.c | 102 +--
1 file changed, 43 insertions(+), 59 deletions(-)
diff --git a/arch/arm/mach-davinci/board-da850-evm.c
b/arch/arm/mach-davinci/board-da850-evm.c
index 32ee3f8..6c172b3 100644
--- a/arch/arm/mach-davinci/board-da850-evm.c
+++ b/arch/arm/mach-davinci/board-da850-evm.c
@@ -347,13 +347,13 @@ static inline void da850_evm_setup_nor_nand(void)
if (!HAS_MMC) {
ret = davinci_cfg_reg_list(da850_evm_nand_pins);
if (ret)
- pr_warning(da850_evm_init: nand mux setup failed:
- %d\n, ret);
+ pr_warn(%s: nand mux setup failed: %d\n,
My preference is to have the acronyms capitalized, so I'd changed to
NAND, while at it.
+ __func__, ret);
ret = davinci_cfg_reg_list(da850_evm_nor_pins);
if (ret)
- pr_warning(da850_evm_init: nor mux setup failed: %d\n,
- ret);
+ pr_warn(%s: nor mux setup failed: %d\n,
... and to NOR here.
@@ -688,14 +688,14 @@ static int da850_evm_bb_expander_setup(struct i2c_client
*client,
da850_evm_bb_keys_init(gpio);
ret = platform_device_register(da850_evm_bb_keys_device);
if (ret) {
- pr_warning(Could not register baseboard GPIO expander keys);
+ pr_warn(Could not register baseboard GPIO expander keys);
goto io_exp_setup_sw_fail;
}
da850_evm_bb_leds_init(gpio);
ret = platform_device_register(da850_evm_bb_leds_device);
if (ret) {
- pr_warning(Could not register baseboard GPIO expander LEDS);
+ pr_warn(Could not register baseboard GPIO expander LEDS);
It's LEDs.
@@ -1060,21 +1060,19 @@ static int __init da850_evm_config_emac(void)
}
if (ret)
- pr_warning(da850_evm_init: cpgmac/rmii mux setup failed: %d\n,
- ret);
+ pr_warn(%s: cpgmac/rmii mux setup failed: %d\n,
I'd have changed to CPGMAC/RMII.
@@ -1085,8 +1083,7 @@ static int __init da850_evm_config_emac(void)
ret = da8xx_register_emac();
if (ret)
- pr_warning(da850_evm_init: emac registration failed: %d\n,
- ret);
+ pr_warn(%s: emac registration failed: %d\n, __func__, ret);
... and to EMAC here.
@@ -1438,57 +1435,53 @@ static __init void da850_evm_init(void)
ret = pmic_tps65070_init();
if (ret)
- pr_warning(da850_evm_init: TPS65070 PMIC init failed: %d\n,
- ret);
+ pr_warn(%s: TPS65070 PMIC init failed: %d\n, __func__, ret);
ret = da850_register_edma(da850_edma_rsv);
if (ret)
- pr_warning(da850_evm_init: edma registration failed: %d\n,
- ret);
+ pr_warn(%s: edma registration failed: %d\n, __func__, ret);
... and to EDMA here.
ret = davinci_cfg_reg_list(da850_i2c0_pins);
if (ret)
- pr_warning(da850_evm_init: i2c0 mux setup failed: %d\n,
- ret);
+ pr_warn(%s: i2c0 mux setup failed: %d\n, __func__, ret);
... and to I2C0 here.
ret = da8xx_register_i2c(0, da850_evm_i2c_0_pdata);
if (ret)
- pr_warning(da850_evm_init: i2c0 registration failed: %d\n,
- ret);
+ pr_warn(%s: i2c0 registration failed: %d\n, __func__, ret);
... and here.
if (HAS_MMC) {
ret = davinci_cfg_reg_list(da850_evm_mmcsd0_pins);
if (ret)
- pr_warning(da850_evm_init: mmcsd0 mux setup failed:
-%d\n, ret);
+ pr_warn(%s: mmcsd0 mux setup failed: %d\n,
... and to MMCSD0 here.
[...]
if (ret)
- pr_warning(da850_evm_init: mmcsd0 registration failed:
-%d\n, ret);
+ pr_warn(%s: mmcsd0 registration failed: %d\n,
... and here.
+ __func__, ret);
ret = da850_wl12xx_init();
if (ret)
-
Re: [PATCH v3 2/6] ARM: davinci: Changed pr_warning() to pr_warn() (part 2)
Hello.
On 14-11-2012 4:33, Robert Tivy wrote:
Also, while modifying those pr_warning() calls I changed hardcoded
function names to use '%s:, __func__' instead
Signed-off-by: Robert Tivy rt...@ti.com
---
Clean up files that will be otherwise modified in subsequent patch.
Applies to v3.7-rc2 tag (commit 6f0c0580b70c89094b3422ba81118c7b959c7556) of
Linus' mainline kernel at git.kernel.org.
arch/arm/mach-davinci/board-omapl138-hawk.c | 30 ++-
1 file changed, 11 insertions(+), 19 deletions(-)
Taksing of separation of board and SoC specific changes, this patch
shouldn't have been separated from patch 1 at all -- since it's two boards
built around the same chip, OMAP-L138...
diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c
b/arch/arm/mach-davinci/board-omapl138-hawk.c
index dc1208e..8aea169 100644
--- a/arch/arm/mach-davinci/board-omapl138-hawk.c
+++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
@@ -48,8 +48,7 @@ static __init void omapl138_hawk_config_emac(void)
val = ~BIT(8);
ret = davinci_cfg_reg_list(omapl138_hawk_mii_pins);
if (ret) {
- pr_warning(%s: cpgmac/mii mux setup failed: %d\n,
- __func__, ret);
+ pr_warn(%s: cpgmac/mii mux setup failed: %d\n, __func__, ret);
I'd have preferred this as CPGMAC/MII. Almost all other acronyms in the
messages are capitalized.
return;
}
@@ -61,8 +60,7 @@ static __init void omapl138_hawk_config_emac(void)
ret = da8xx_register_emac();
if (ret)
- pr_warning(%s: emac registration failed: %d\n,
- __func__, ret);
+ pr_warn(%s: emac registration failed: %d\n, __func__, ret);
... and EMAC here.
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: State of DM6446 EVM in mainline
Hello. On 11/13/2012 09:35 PM, Sergei Shtylyov wrote: I got no further than IP-Config when booting the DM6446 EVM board with both current linux.git tree and linux-davinci.git from gitorious.org. After a significant timeout I get VFS: Unable to mount root via NFS, trying floppy. VFS: Cannot open root device nfs or unknown-block(2,0): error -6 Please append a correct root= boot option; here are the available partitions: Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(2,0) [usual backtrace follows] Oops, it was due to Fedora's firewall, damn it! Moreover, I have noticed the following in the boot log: serial8250 serial8250.0: unable to register port at index 1 (IO0 MEM1c20400 IRQ4 1): -22 serial8250 serial8250.0: unable to register port at index 2 (IO0 MEM1c20800 IRQ4 2): -22 These still appear, of course... Sekhar, have you boot tested this platform recently? WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
State of DM6446 EVM in mainline
Hello. I got no further than IP-Config when booting the DM6446 EVM board with both current linux.git tree and linux-davinci.git from gitorious.org. After a significant timeout I get VFS: Unable to mount root via NFS, trying floppy. VFS: Cannot open root device nfs or unknown-block(2,0): error -6 Please append a correct root= boot option; here are the available partitions: Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(2,0) [usual backtrace follows] Moreover, I have noticed the following in the boot log: serial8250 serial8250.0: unable to register port at index 1 (IO0 MEM1c20400 IRQ4 1): -22 serial8250 serial8250.0: unable to register port at index 2 (IO0 MEM1c20800 IRQ4 2): -22 Sekhar, have you boot tested this platform recently? WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2 1/6] ARM: davinci: Changed pr_warning() to pr_warn()
Hello. It's not a good idea to send multiple patches with the same subject. Actually, in this case it's worth merging all 4 patches into one. On 26-10-2012 0:35, Robert Tivy wrote: Also, while modifying those pr_warning() calls I changed hardcoded function names to use '%s:, __func__' instead Signed-off-by: Robert Tivy rt...@ti.com --- Clean up files that will be otherwise modified in subsequent patch. Applies to v3.5 tag (commit 28a33cbc24e4256c143dce96c7d93bf423229f92) of Linus' mainline kernel at git.kernel.org. 3.5 is too old. Why not to 3.6 or even 3.7-rc2? diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c index 0149fb4..bbb3c73 100644 --- a/arch/arm/mach-davinci/board-da850-evm.c +++ b/arch/arm/mach-davinci/board-da850-evm.c [...] @@ -1046,21 +1046,19 @@ static int __init da850_evm_config_emac(void) } if (ret) - pr_warning(da850_evm_init: cpgmac/rmii mux setup failed: %d\n, - ret); + pr_warn(%s: cpgmac/rmii mux setup failed: %d\n, + __func__, ret); /* configure the CFGCHIP3 register for RMII or MII */ __raw_writel(val, cfg_chip3_base); ret = davinci_cfg_reg(DA850_GPIO2_6); if (ret) - pr_warning(da850_evm_init:GPIO(2,6) mux setup - failed\n); + pr_warn(%s:GPIO(2,6) mux setup failed\n, __func__); Worth inserting space after colon here. WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2] ARM: dm365: replace V4L2_OUT_CAP_CUSTOM_TIMINGS with V4L2_OUT_CAP_DV_TIMINGS
On 24.10.2012 15:19, Sekhar Nori wrote: This patch replaces V4L2_OUT_CAP_CUSTOM_TIMINGS macro with V4L2_OUT_CAP_DV_TIMINGS. As V4L2_OUT_CAP_CUSTOM_TIMINGS is being phased out. Signed-off-by: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com Cc: Sekhar Nori nsek...@ti.com Cc: Sergei Shtylyov sshtyl...@mvista.com Patches for mach-davinci should have a 'davinci:' prefix after 'ARM:'. Can you please resend with that fixed? Also, the patch is for DM365 EVM board, not DM365 SoC. Thanks, Sekhar WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH RESEND] ARM: dm365: replace V4L2_OUT_CAP_CUSTOM_TIMINGS with V4L2_OUT_CAP_DV_TIMINGS
Hello.
On 22-10-2012 16:12, Prabhakar Lad wrote:
From: Lad, Prabhakar prabhakar@ti.com
This patch replaces V4L2_OUT_CAP_CUSTOM_TIMINGS macro with
V4L2_OUT_CAP_DV_TIMINGS. As V4L2_OUT_CAP_CUSTOM_TIMINGS is being phased
out.
Signed-off-by: Lad, Prabhakar prabhakar@ti.com
Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com
Cc: Sekhar Nori nsek...@ti.com
---
Resending the patch since, it didn't reach the DLOS mailing list.
This patch is based on the following patch series,
ARM: davinci: dm365 EVM: add support for VPBE display
(https://patchwork.kernel.org/patch/1295071/)
arch/arm/mach-davinci/board-dm365-evm.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-davinci/board-dm365-evm.c
b/arch/arm/mach-davinci/board-dm365-evm.c
index 2924d61..771abb5 100644
--- a/arch/arm/mach-davinci/board-dm365-evm.c
+++ b/arch/arm/mach-davinci/board-dm365-evm.c
@@ -514,7 +514,7 @@ static struct vpbe_output dm365evm_vpbe_outputs[] = {
.index = 1,
.name = Component,
.type = V4L2_OUTPUT_TYPE_ANALOG,
- .capabilities = V4L2_OUT_CAP_CUSTOM_TIMINGS,
+ .capabilities = V4L2_OUT_CAP_DV_TIMINGS,
Why this extra space after '='?
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2 2/2] ARM: davinci: enable SRAM ping ping buffering on DA850
Hello. On 11-10-2012 23:33, Matt Porter wrote: Passes the DA850 shared SRAM gen_pool to the McASP driver and enables the ping-pong buffer DMA support. Here ping-pong is correct but the subject have a typo. Signed-off-by: Matt Porter mpor...@ti.com WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v6] Enable USB peripheral mode on dm365 EVM
Hello. On 10-10-2012 14:33, Constantine Shulyupin wrote: From: Constantine Shulyupin co...@makelinux.com Sets USB PHY clock source to 24 MHz clock and call USB configuration from board initialization. Tested with OTG configuration, usb gadget g_zero on DM365 EVM connected to PC. References: Definition of USB_PHY_CTRL and PHYCLKFREQ: - http://www.makelinux.com/lib/ti/DM36x_ARM/doc-141 Original patch by miguel.agui...@ridgerun.com three years ago: - http://www.mail-archive.com/davinci-linux-open-source@linux.davincidsp.com/msg14741.html Signed-off-by: Constantine Shulyupin co...@makelinux.com --- Note: Changelog Changes since v5 http://www.spinics.net/lists/kernel/msg1413120.html accordingy feedback of nsek...@ti.com http://www.spinics.net/lists/kernel/msg1414914.html - phy configuration moved to drivers/usb/musb/davinci.c - USB_OTG configuration is submitted in separated patch: http://www.spinics.net/lists/kernel/msg1414964.html - Setting current limit to 1000 mA. Any way the current is limited to 510 mA in davinci_setup_usb. Changes since v4 http://www.spinics.net/lists/kernel/msg1412995.html - removed fix of dev_info in musb_init_controller Changes since v3 http://www.spinics.net/lists/kernel/msg1412544.html: - removed optional altering of pr_info Changes since v1 http://marc.info/?l=linux-kernelm=130894150803661w=2: - removed optional code and reordered - removed alternation of GPIO33, which is multiplexed with DRVVBUS, because is not need for peripheral USB This patch is based on code from projects Arago, Angstom and RidgeRun. --- arch/arm/mach-davinci/board-dm365-evm.c |2 ++ drivers/usb/musb/davinci.c |3 +++ drivers/usb/musb/davinci.h |1 + 3 files changed, 6 insertions(+) diff --git a/arch/arm/mach-davinci/board-dm365-evm.c b/arch/arm/mach-davinci/board-dm365-evm.c index 688a9c5..ba5ffc1 100644 --- a/arch/arm/mach-davinci/board-dm365-evm.c +++ b/arch/arm/mach-davinci/board-dm365-evm.c @@ -38,6 +38,7 @@ #include mach/mmc.h #include mach/nand.h #include mach/keyscan.h +#include mach/usb.h #include media/tvp514x.h @@ -610,6 +611,7 @@ static __init void dm365_evm_init(void) dm365_init_spi0(BIT(0), dm365_evm_spi_info, ARRAY_SIZE(dm365_evm_spi_info)); + davinci_setup_usb(1000, 8); } MACHINE_START(DAVINCI_DM365_EVM, DaVinci DM365 EVM) You need to split the patch at this point. Above part should be applied to the DaVinci tree, below part to the MUSB tree. diff --git a/drivers/usb/musb/davinci.c b/drivers/usb/musb/davinci.c index 472c8b4..af09ebf 100644 --- a/drivers/usb/musb/davinci.c +++ b/drivers/usb/musb/davinci.c @@ -428,6 +428,9 @@ static int davinci_musb_init(struct musb *musb) __raw_writel(deepsleep, DM355_DEEPSLEEP); } + if (machine_is_davinci_dm365_evm()) + writel(readl(USB_PHY_CTRL) | USBPHY_CLKFREQ_24MHZ, USB_PHY_CTRL); I'd put that to the board file instead, like in board-da830-evm.c WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [RFC PATCH 08/10] ARM:davinci - migrating to use davinci_common_clk_init
Hello.
On 18-09-2012 22:35, Murali Karicheri wrote:
The common clk code uses a new function davinci_common_clk_init()
defined in drivers/clk/davinci/davinci-clock.c to initialize the
clk drivers. This function is now invoked in time.c as part of
davinci_timer_init(). Currently davinci_clk_init() is called from
davinci_common_init() which is too early to initialize common clk
drivers. Also include pll.h instead of clock.h in some of the source
files.
Signed-off-by: Murali Karicheri m-kariche...@ti.com
diff --git a/arch/arm/mach-davinci/common.c b/arch/arm/mach-davinci/common.c
index 64b0f65..f854296 100644
--- a/arch/arm/mach-davinci/common.c
+++ b/arch/arm/mach-davinci/common.c
[...]
@@ -106,7 +108,9 @@ void __init davinci_common_init(struct davinci_soc_info
*soc_info)
goto err;
if (davinci_soc_info.cpu_clks) {
+#ifndef CONFIG_COMMON_CLK
Shouldn't you enclose in theis #ifndef the whole *if* statement?
ret = davinci_clk_init(davinci_soc_info.cpu_clks);
+#endif
if (ret != 0)
goto err;
[...]
diff --git a/arch/arm/mach-davinci/sleep.S b/arch/arm/mach-davinci/sleep.S
index d4e9316..5c04a7c 100644
--- a/arch/arm/mach-davinci/sleep.S
+++ b/arch/arm/mach-davinci/sleep.S
@@ -183,6 +187,7 @@ ENDPROC(davinci_cpu_suspend)
*r1: contains virtual base for DDR2 Power and Sleep controller (PSC)
*r2: contains PSC number for DDR2
*/
+
Unrelated/unnecessary change?
ENTRY(davinci_ddr_psc_config)
/* Set next state in mdctl for DDR2 */
mov r6, #MDCTL
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH] davinci: vpif: capture/display: fix race condition
Hello. On 09/14/2012 05:53 PM, Prabhakar Lad wrote: From: Lad, Prabhakar prabhakar@ti.com channel_first_int[][] variable is used as a flag for the ISR, This flag was being set after enabling the interrupts, There where suitaions when the isr ocuurend even before the flag was set s/suitaions/situations/, s/ocuurend/occured/ dues to which it was causing the applicaiotn hang. Application. This patch sets channel_first_int[][] flag just before enabling the interrupt. Reported-by: David Oleszkiewicz doles...@adsyscontrols.com Signed-off-by: Lad, Prabhakar prabhakar@ti.com Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com Cc: Hans Verkuil hans.verk...@cisco.com WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 6/6] i2c: davinci: use devm_ functions
Hello.
On 08/25/2012 09:41 PM, Julia Lawall wrote:
From: Julia Lawall julia.law...@lip6.fr
The various devm_ functions allocate memory that is released when a driver
detaches. This patch uses these functions for data that is allocated in
the probe function of a platform device and is only freed in the remove
function.
The call to platform_get_resource is moved down to the call to
devm_request_and_ioremap that uses it.
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
Not compiled.
I see. :-)
I was not sure what to do with the comment on the first line, so I just
left it where it is.
I was also concerned about the calls to get_device and put_device, and
whether they would cause pdev-dev to be freed before the devm-allocated
objects were freed. Most other platform drivers don't seem to have these
calls.
drivers/i2c/busses/i2c-davinci.c | 51
+--
1 file changed, 12 insertions(+), 39 deletions(-)
diff --git a/drivers/i2c/busses/i2c-davinci.c
b/drivers/i2c/busses/i2c-davinci.c
index b6185dc..c8e9c87 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -647,30 +647,16 @@ static int davinci_i2c_probe(struct platform_device
*pdev)
int r;
/* NOTE: driver uses the static register mapping */
- mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!mem) {
- dev_err(pdev-dev, no mem resource?\n);
- return -ENODEV;
- }
-
irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
if (!irq) {
dev_err(pdev-dev, no irq resource?\n);
return -ENODEV;
}
- ioarea = request_mem_region(mem-start, resource_size(mem),
- pdev-name);
- if (!ioarea) {
- dev_err(pdev-dev, I2C region already claimed\n);
- return -EBUSY;
- }
Shouldn't you have dropped the 'ioarea' variable? It should be unused now...
@@ -699,14 +685,15 @@ static int davinci_i2c_probe(struct platform_device
*pdev)
[...]
- dev-base = ioremap(mem-start, resource_size(mem));
+ mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
Why you dropped check form 'mem' being NULL?
+ dev-base = devm_request_and_ioremap(pdev-dev, mem);
if (!dev-base) {
r = -EBUSY;
goto err_mem_ioremap;
@@ -714,16 +701,17 @@ static int davinci_i2c_probe(struct platform_device
*pdev)
i2c_davinci_init(dev);
- r = request_irq(dev-irq, i2c_davinci_isr, 0, pdev-name, dev);
+ r = devm_request_irq(pdev-dev, dev-irq, i2c_davinci_isr, 0,
+ pdev-name, dev);
if (r) {
dev_err(pdev-dev, failure requesting irq %i\n, dev-irq);
- goto err_unuse_clocks;
+ goto err_mem_ioremap;
The label no longer corresponds the failure happening. Perhaps it's better to
leave 'err_unuse_clocks'...
}
r = i2c_davinci_cpufreq_register(dev);
if (r) {
dev_err(pdev-dev, failed to register cpufreq\n);
- goto err_free_irq;
+ goto err_mem_ioremap;
Ditto...
@@ -740,26 +728,18 @@ static int davinci_i2c_probe(struct platform_device
*pdev)
r = i2c_add_numbered_adapter(adap);
if (r) {
dev_err(pdev-dev, failure adding adapter\n);
- goto err_free_irq;
+ goto err_mem_ioremap;
Ditto...
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v4] da8xx-fb: add 24bpp LCD configuration support
Hello.
On 08-08-2012 19:35, Manjunathappa, Prakash wrote:
LCD controller on am335x supports 24bpp raster configuration in addition
to ones on da850. LCDC also supports 24bpp in unpacked format having
ARGB: 32bpp format data in DDR, but it doesn't interpret alpha
component of the data.
Signed-off-by: Manjunathappa, Prakash prakash...@ti.com
Cc: Anatolij Gustschin ag...@denx.de
[...]
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 7ae9d53..1abcfa9 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
[...]
@@ -499,6 +501,9 @@ static int lcd_cfg_frame_buffer(struct da8xx_fb_par *par,
u32 width, u32 height,
{
u32 reg;
+ if ((bpp 16) (lcd_revision == LCD_VERSION_1))
Parens around operands of not necessary.
@@ -542,6 +547,12 @@ static int lcd_cfg_frame_buffer(struct da8xx_fb_par *par,
u32 width, u32 height,
reg = lcdc_read(LCD_RASTER_CTRL_REG) ~(1 8);
if (raster_order)
reg |= LCD_RASTER_ORDER;
+
+ if (bpp == 24)
+ reg |= LCD_V2_TFT_24BPP_MODE;
+ else if (bpp == 32)
+ reg |= (LCD_V2_TFT_24BPP_MODE | LCD_V2_TFT_24BPP_UNPACK);
+
This asks to be a *switch* statement.
lcdc_write(reg, LCD_RASTER_CTRL_REG);
switch (bpp) {
@@ -549,6 +560,8 @@ static int lcd_cfg_frame_buffer(struct da8xx_fb_par *par,
u32 width, u32 height,
case 2:
case 4:
case 16:
+ case 24:
+ case 32:
par-palette_sz = 16 * 2;
break;
@@ -578,13 +591,36 @@ static int fb_setcolreg(unsigned regno, unsigned red,
unsigned green,
if (info-fix.visual == FB_VISUAL_DIRECTCOLOR)
return 1;
- if (info-var.bits_per_pixel == 4) {
- if (regno 15)
- return 1;
+ if ((info-var.bits_per_pixel 16) (lcd_revision == LCD_VERSION_1))
Parens around operands of not necessary.
+ switch (info-fix.visual) {
+ case FB_VISUAL_TRUECOLOR:
+ red = CNVT_TOHW(red, info-var.red.length);
+ green = CNVT_TOHW(green, info-var.green.length);
+ blue = CNVT_TOHW(blue, info-var.blue.length);
+ break;
+ case FB_VISUAL_PSEUDOCOLOR:
+ if (info-var.bits_per_pixel == 4) {
+ if (regno 15)
+ return -EINVAL;
+
+ if (info-var.grayscale) {
+ pal = regno;
+ } else {
+ red = 4;
+ green = 8;
+ blue = 12;
+
+ pal = (red 0x0f00);
+ pal |= (green 0x00f0);
+ pal |= (blue 0x000f);
Parens not needed.
+ }
+ if (regno == 0)
+ pal |= 0x2000;
+ palette[regno] = pal;
+
+ } else if (info-var.bits_per_pixel == 8) {
This asks to be a *switch* statement.
@@ -842,6 +877,9 @@ static int fb_check_var(struct fb_var_screeninfo *var,
{
int err = 0;
+ if ((var-bits_per_pixel 16) (lcd_revision == LCD_VERSION_1))
Parens around operands of not necessary.
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 1/1] media/video: vpif: fixing function name start to vpif_config_params
Hello.
On 02-08-2012 11:40, Dror Cohen wrote:
diff --git a/drivers/media/video/davinci/vpif_capture.h
b/drivers/media/video/davinci/vpif_capture.h
index a693d4e..8863de1 100644
--- a/drivers/media/video/davinci/vpif_capture.h
+++ b/drivers/media/video/davinci/vpif_capture.h
@@ -144,7 +144,7 @@ struct vpif_device {
struct v4l2_subdev **sd;
};
-struct vpif_config_params {
+struct config_vpif_params_t {
IMO, '_t' postfix is used only for *typedef* names.
diff --git a/drivers/media/video/davinci/vpif_display.h
b/drivers/media/video/davinci/vpif_display.h
index 56879d1..3e14807 100644
--- a/drivers/media/video/davinci/vpif_display.h
+++ b/drivers/media/video/davinci/vpif_display.h
@@ -154,7 +154,7 @@ struct vpif_device {
};
-struct vpif_config_params {
+struct config_vpif_params_t {
Same comment.
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 0/1] media/video: vpif: fixing function name start to vpif_config_params
Hello. On 02-08-2012 11:40, Dror Cohen wrote: This patch address the issue that a function named config_vpif_params should be vpif_config_params. This, however, conflicts with two structures defined already. Function names shouldn't conflict with the structure tags. Structure tags always follow 'struct' keyword and are only valid in this context. So I changed the structures to config_vpif_params_t (origin was vpif_config_params) WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2 1/6] rtc: omap: kicker mechanism support
Hello. On 25-07-2012 10:12, Afzal Mohammed wrote: OMAP RTC IP can have kicker feature. This prevents spurious writes to register. To write to registers kicker lock has to be released. Procedure to do it as follows, 1. write to kick0 register, 0x83e70b13 2. write to kick1 register, 0x95a4f1e0 Writing value other than 0x83e70b13 to kick0 enables write locking, more details about kicker mechanism can be found in section 20.3.3.5.3 of AM335X TRM @www.ti.com/am335x Here id table information is added and is used to distinguish those that require kicker handling and the ones that doesn't need it. There are more features in the newer IP's compared to legacy ones other than kicker, which driver currently doesn't handle, supporting additional features would be easier with the addition of id table. Older IP (of OMAP1) doesn't have revision register as per TRM, so revision register can't be relied always to find features, hence id table is being used. Signed-off-by: Afzal Mohammed af...@ti.com --- v2: Use device name da830-rtc instead of am1808-rtc Newly added register name made similar to that existing in the driver Better commit message description drivers/rtc/rtc-omap.c | 39 ++- 1 files changed, 38 insertions(+), 1 deletions(-) diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c index 0b614e3..8afbc2e 100644 --- a/drivers/rtc/rtc-omap.c +++ b/drivers/rtc/rtc-omap.c @@ -38,6 +38,8 @@ * the SoC). See the BOARD-SPECIFIC CUSTOMIZATION comment. */ +#defineDRIVER_NAME omap_rtc + #define OMAP_RTC_BASE 0xfffb4800 /* RTC registers */ @@ -64,6 +66,9 @@ #define OMAP_RTC_COMP_MSB_REG 0x50 #define OMAP_RTC_OSC_REG 0x54 +#define OMAP_RTC_KICK0_REG 0x6c +#define OMAP_RTC_KICK1_REG 0x70 + /* OMAP_RTC_CTRL_REG bit fields: */ #define OMAP_RTC_CTRL_SPLIT (17) #define OMAP_RTC_CTRL_DISABLE (16) @@ -88,11 +93,19 @@ #define OMAP_RTC_INTERRUPTS_IT_ALARM(13) #define OMAP_RTC_INTERRUPTS_IT_TIMER(12) +/* OMAP_RTC_KICKER values */ +#defineKICK0_VALUE (0x83e70b13) +#defineKICK1_VALUE (0x95a4f1e0) Parens not needed around simple literals. static void __iomem *rtc_base; #define rtc_read(addr)__raw_readb(rtc_base + (addr)) #define rtc_write(val, addr) __raw_writeb(val, rtc_base + (addr)) +#define rtc_writel(val, addr) writel(val, rtc_base + (addr)) + Why not __raw_writel() like the above functions? WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2 6/6] arm/dts: am33xx rtc node
Hello.
On 25-07-2012 10:12, Afzal Mohammed wrote:
Add AM33xx rtc node.
Signed-off-by: Afzal Mohammed af...@ti.com
---
v2:
Use compatible as ti,da830-rtc instead of ti,am1808-rtc
arch/arm/boot/dts/am33xx.dtsi |5 +
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index bd0cff3..e1ed72d 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -159,5 +159,10 @@
compatible = ti,omap3-wdt;
ti,hwmods = wd_timer2;
};
+
+ rtc@44e3e000 {
Address postfix in the node name without reg property?
+ compatible = ti,da830-rtc;
+ ti,hwmods = rtc;
+ };
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2 6/6] arm/dts: am33xx rtc node
Hello.
On 07/25/2012 06:09 PM, Mohammed, Afzal wrote:
+ rtc@44e3e000 {
Address postfix in the node name without reg property?
As per [1], The unit-address is included if the node describes
a device with an address.
Which in this case it doesn't.
Here even though reg property is not present, as via hwmod
(see below) it is getting address, isn't it better to have it
I think not.
+ compatible = ti,da830-rtc;
+ ti,hwmods = rtc;
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 2/6] ARM: davinci: remove rtc kicker release
Hello.
On 23-07-2012 17:42, Afzal Mohammed wrote:
rtc-omap driver is now capable of handling kicker mechanism,
hence remove kicker handling at platform level, instead
provide proper device name so that driver can handle kicker
mechanism by itself
Signed-off-by: Afzal Mohammed af...@ti.com
---
arch/arm/mach-davinci/devices-da8xx.c | 13 +
1 files changed, 1 insertions(+), 12 deletions(-)
diff --git a/arch/arm/mach-davinci/devices-da8xx.c
b/arch/arm/mach-davinci/devices-da8xx.c
index d1624a3..c915bff 100644
--- a/arch/arm/mach-davinci/devices-da8xx.c
+++ b/arch/arm/mach-davinci/devices-da8xx.c
@@ -679,7 +679,7 @@ static struct resource da8xx_rtc_resources[] = {
};
static struct platform_device da8xx_rtc_device = {
- .name = omap_rtc,
+ .name = am1808-rtc,
Why not da8xx-rtc. Kick registers exist startting with
DA830/OMAP-L137/AM1707, not only on AM1808.
.id = -1,
.num_resources = ARRAY_SIZE(da8xx_rtc_resources),
.resource = da8xx_rtc_resources,
@@ -688,17 +688,6 @@ static struct platform_device da8xx_rtc_device = {
int da8xx_register_rtc(void)
{
int ret;
- void __iomem *base;
-
- base = ioremap(DA8XX_RTC_BASE, SZ_4K);
- if (WARN_ON(!base))
- return -ENOMEM;
-
- /* Unlock the rtc's registers */
- __raw_writel(0x83e70b13, base + 0x6c);
- __raw_writel(0x95a4f1e0, base + 0x70);
-
- iounmap(base);
ret = platform_device_register(da8xx_rtc_device);
if (!ret)
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v2] da8xx-fb: enable sync lost interrupt
Hello.
On 24-07-2012 7:43, Manjunathappa, Prakash wrote:
Patch enables sync lost interrupt and interrupt handler already
takes care to handle it.
Signed-off-by: Manjunathappa, Prakash prakash...@ti.com
---
Since v1:
Minor nit, replaced spaces with tabs.
drivers/video/da8xx-fb.c |7 +--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 47118c7..8163832 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
[...]
@@ -718,6 +719,7 @@ static irqreturn_t lcdc_irq_handler_rev02(int irq, void
*arg)
u32 reg_int;
if ((stat LCD_SYNC_LOST) (stat LCD_FIFO_UNDERFLOW)) {
+ pr_err(LCDC sync lost or underflow error occurred\n);
You write or here. Perhaps it should be || instead of above?
lcd_disable_raster();
lcdc_write(stat, LCD_MASKED_STAT_REG);
lcd_enable_raster();
@@ -773,6 +775,7 @@ static irqreturn_t lcdc_irq_handler_rev01(int irq, void
*arg)
u32 reg_ras;
if ((stat LCD_SYNC_LOST) (stat LCD_FIFO_UNDERFLOW)) {
+ pr_err(LCDC sync lost or underflow error occurred\n);
Same comment.
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH RESEND] video: da8xx-fb: enable sync lost interrupt
Hello. On 07/18/2012 07:30 PM, Manjunathappa, Prakash wrote: Patch enables sync lost interrupt and interrupt handler already takes care to handle it. Signed-off-by: Manjunathappa, Prakash prakash...@ti.com --- Resending as my earlier patch seems like not reached fbdev mailing list. drivers/video/da8xx-fb.c |5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c index 88e98ea..e9d2f6e 100644 --- a/drivers/video/da8xx-fb.c +++ b/drivers/video/da8xx-fb.c @@ -54,6 +54,7 @@ #define LCD_DMA_BURST_8 0x3 #define LCD_DMA_BURST_16 0x4 #define LCD_V1_END_OF_FRAME_INT_ENA BIT(2) +#define LCD_V1_SYNC_LOST_ENABIT(5) Please indent the value with tabs, as the others. #define LCD_V2_END_OF_FRAME0_INT_ENA BIT(8) #define LCD_V2_END_OF_FRAME1_INT_ENA BIT(9) #define LCD_DUAL_FRAME_BUFFER_ENABLE BIT(0) WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 1/2] video: da8xx-fb: configure FIFO threshold to reduce underflow errors
Hello.
On 11-07-2012 19:14, Manjunathappa, Prakash wrote:
Patch works around the below silicon errata:
During LCDC initialization, there is the potential for a FIFO
underflow condition to occur. A FIFO underflow condition
occurs when the input FIFO is completely empty and the LCDC
raster controller logic that drives data to the output pins
attempts to fetch data from the FIFO. When a FIFO underflow
condition occurs, incorrect data will be driven out on the
LCDC data pins.
Software should poll the FUF bit field in the LCD_STAT register
to check if an error condition has occurred or service the
interrupt if FUF_EN is enabled when FUF occurs. If the FUF bit
field has been set to 1, this will indicate an underflow
condition has occurred and then the software should execute a
reset of the LCDC via the LPSC.
This problem may occur if the LCDC FIFO threshold size
(LCDDMA_CTRL[TH_FIFO_READY]) is left at its default value after
reset. Increasing the FIFO threshold size will reduce or
eliminate underflows. Setting the threshold size to 256 double
words or larger is recommended.
Above issue is described in section 2.1.3 of silicon errata
http://www.ti.com/lit/er/sprz313e/sprz313e.pdf
Signed-off-by: Rajashekhara, Sudhakar sudhakar@ti.com
Signed-off-by: Manjunathappa, Prakash prakash...@ti.com
---
drivers/video/da8xx-fb.c | 15 +++
include/video/da8xx-fb.h |3 +++
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
index 47118c7..2010dd7 100644
--- a/drivers/video/da8xx-fb.c
+++ b/drivers/video/da8xx-fb.c
@@ -344,8 +344,8 @@ static void lcd_blit(int load_mode, struct da8xx_fb_par
*par)
lcd_enable_raster();
}
-/* Configure the Burst Size of DMA */
-static int lcd_cfg_dma(int burst_size)
+/* Configure the Burst Size and fifo threhold of DMA */
+static int lcd_cfg_dma(int burst_size, int fifo_th)
One space too much.
{
u32 reg;
@@ -719,8 +722,10 @@ static irqreturn_t lcdc_irq_handler_rev02(int irq, void
*arg)
if ((stat LCD_SYNC_LOST) (stat LCD_FIFO_UNDERFLOW)) {
lcd_disable_raster();
+ clk_disable(par-lcdc_clk);
lcdc_write(stat, LCD_MASKED_STAT_REG);
Will LCDC register wrtite work without LCDC clock?
lcd_enable_raster();
+ clk_enable(par-lcdc_clk);
} else if (stat LCD_PL_LOAD_DONE) {
/*
* Must disable raster before changing state of any control bit.
@@ -774,8 +779,10 @@ static irqreturn_t lcdc_irq_handler_rev01(int irq, void
*arg)
if ((stat LCD_SYNC_LOST) (stat LCD_FIFO_UNDERFLOW)) {
lcd_disable_raster();
+ clk_disable(par-lcdc_clk);
lcdc_write(stat, LCD_STAT_REG);
Same question.
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v3 04/13] davinci: vpif: fix setting of data width in config_vpif_params() function
Hello. On 25-06-2012 15:07, Manjunath Hadli wrote: fix setting of data width in config_vpif_params() function, which was wrongly set. Signed-off-by: Manjunath Hadlimanjunath.ha...@ti.com Signed-off-by: Lad, Prabhakarprabhakar@ti.com --- drivers/media/video/davinci/vpif.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/media/video/davinci/vpif.c b/drivers/media/video/davinci/vpif.c index 774bcd3..08fb81f 100644 --- a/drivers/media/video/davinci/vpif.c +++ b/drivers/media/video/davinci/vpif.c @@ -346,7 +346,7 @@ static void config_vpif_params(struct vpif_params *vpifparams, value = regr(reg); /* Set data width */ - value= ((~(unsigned int)(0x3)) + value= ~(((unsigned int)(0x3)) Why not just 0x3u instead of (unsigned int)(0x3)? WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH] usb: musb: davinci: Fix build breakage
Hello. On 24-05-2012 6:25, Jon Povey wrote: This appears to have been broken by commit 5cfb19ac604a68c030b245561f575c2d1bac1d49 Please also specify that commit's summary in parens. For now, fix by hardcoding USB_PHY_CTRL and DM355_DEEPSLEEP Tested on DM365 with defconfig changes. Signed-off-by: Jon Poveyjon.po...@racelogic.co.uk Cc: Sekhar Norinsek...@ti.com Cc: Felipe Balbiba...@ti.com --- This follows from my email DaVinci MUSB driver compile errors on 3.4 I suppose this should go to stable too, but I can't see a nice way to add them as CC in the patch but NOT email them right now when using git send-email. You can do both at once AFAIK. To test on DM365 I had to apply a patch similar to this one: http://linux.omap.com/pipermail/davinci-linux-open-source/2011-July/023212.html That patch wasn't split/recast as needed that's why it wasn't accepted. As a separate issue, MUSB drivers seem quite broken on DM365 in 3.2 and 3.4, but at least with this patch they compile. One step at a time.. Maybe I'll try to test it on DM6446 EVM. Though it doesn'[t support OTG mode... WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v4 1/7] ARM: davinci, intc: Add irq domain support
On 05/22/2012 05:55 PM, Heiko Schocher wrote:
Signed-off-by: Heiko Schocherh...@denx.de
Cc: davinci-linux-open-source@linux.davincidsp.com
Cc: linux-arm-ker...@lists.infradead.org
Cc: devicetree-disc...@lists.ozlabs.org
Cc: Grant Likelygrant.lik...@secretlab.ca
Cc: Sekhar Norinsek...@ti.com
Cc: Wolfgang Denkw...@denx.de
Cc: Sergei Shtylyovsshtyl...@mvista.com
In the patch subject, you'd better specify cp_intc, to avoid any confusion.
Same comment for the next patch...
diff --git a/arch/arm/mach-davinci/cp_intc.c b/arch/arm/mach-davinci/cp_intc.c
index f83152d..bb52807 100644
--- a/arch/arm/mach-davinci/cp_intc.c
+++ b/arch/arm/mach-davinci/cp_intc.c
[...]
@@ -99,18 +101,37 @@ static struct irq_chip cp_intc_irq_chip = {
[...]
+int __init __cp_intc_init(struct device_node *node)
+{
+ u32 num_irq = davinci_soc_info.intc_irq_num;
u8 *irq_prio= davinci_soc_info.intc_irq_prios;
u32 *host_map = davinci_soc_info.intc_host_map;
unsigned num_reg= BITS_TO_LONGS(num_irq);
- int i;
+ int i, irq_base;
davinci_intc_type = DAVINCI_INTC_TYPE_CP_INTC;
davinci_intc_base = ioremap(davinci_soc_info.intc_base, SZ_8K);
+
Empty line not needed.
if (WARN_ON(!davinci_intc_base))
[...]
@@ -165,13 +186,28 @@ void __init cp_intc_init(void)
for (i = 0; host_map[i] != -1; i++)
cp_intc_write(host_map[i], CP_INTC_HOST_MAP(i));
- /* Set up genirq dispatching for cp_intc */
- for (i = 0; i num_irq; i++) {
- irq_set_chip(i,cp_intc_irq_chip);
- set_irq_flags(i, IRQF_VALID | IRQF_PROBE);
- irq_set_handler(i, handle_edge_irq);
+ irq_base = irq_alloc_descs(-1, 0, num_irq, 0);
+ if (irq_base 0) {
+ pr_warn(Couldn't allocate IRQ numbers\n);
+ irq_base = 0;
+ }
+
+ /* create a legacy host */
+ cp_intc_domain = irq_domain_add_legacy(node, num_irq,
+ irq_base, 0,cp_intc_host_ops, NULL);
+
+ if (cp_intc_domain == NULL) {
'if (!cp_int_domain)' please -- to keep the style consistent.
+ pr_err(CP INTC: failed to allocate irq host!\n);
s/CP_INTC/cp_intc/
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: usb, davinci: usb 2.0 problem on an am1808 based board
Hello. On 18-05-2012 14:07, Manjunathappa, Prakash wrote: I do not know how putting delay helped MSC device detection. Can you please check if MUSB is coming up in b_idle state(by $cat /sys/devices/platform/musb-da8xx/musb-hdrc/mode)? State should move to b_peripheral on connecting gadget cable. If you connect MSC device via mini-A connector, state should change to a_host. OTG timer is responsible for above state changes, can you please check if below changes are present? diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c index 4da7492..a1a692e 100644 --- a/drivers/usb/musb/da8xx.c +++ b/drivers/usb/musb/da8xx.c @@ -76,6 +76,7 @@ #define DA8XX_INTR_TX_SHIFT0 #define DA8XX_INTR_TX_MASK (DA8XX_USB_TX_EP_MASK DA8XX_INTR_TX_SHIFT) +#define A_WAIT_BCON_TIMEOUT 1100/* in ms */ #define DA8XX_MENTOR_CORE_OFFSET 0x400 #define CFGCHIP2 IO_ADDRESS(DA8XX_SYSCFG0_BASE + DA8XX_CFGCHIP2_REG) @@ -443,6 +444,7 @@ static int da8xx_musb_init(struct musb *musb) rev, __raw_readl(CFGCHIP2), musb_readb(reg_base, DA8XX_USB_CTRL_REG)); + musb-a_wait_bcon = A_WAIT_BCON_TIMEOUT; musb-isr = da8xx_musb_interrupt; return 0; fail: This change shouldn't be needed as musb-a_wait_bcon is set in musb_core.c::allocate_instance(). Thanks, Prakash WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH] arm: davinci: pdctl next bit position incorrect
Hello. On 26-04-2012 21:34, Sekhar Nori wrote: On 04/19/2012 08:52 PM, Sekhar Nori wrote: The PDCTL NEXT bit is incorrectly set to bit 1 instead of bit 0. This patch fixes this issue Signed-off-by: Murali Karicherim-kariche...@ti.com Applying this for v3.5. But why not to 3.4? It concerns turning on DSP domain on DM644x. Yes, but I think as you noted sometime before, no one is really depending on kernel to turn on DSP domain on DM644x. Looks like you're right (though I don't remember noting it :-). Its probably being done from U-Boot or ROM code. This is not a v3.4 regression either. U-Boot does it only when DSPLINK support is not desired. Okay. Since DSPLink existed long before this code to turn on domain was introduced by Murali, The code to turn on DSP domain wasn't introduced by Murali, it was there originally. Ah, you probably mean that the code was checking PDSTAT0, not PDSTAT1 before doing the DSP domain switch? it must be having its own private code to do this. So, looks like this should not really be a regression for anyone. It seems you're right. Thanks, Sekhar WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH] arm: davinci: pdctl next bit position incorrect
Hello. On 04/19/2012 08:52 PM, Sekhar Nori wrote: The PDCTL NEXT bit is incorrectly set to bit 1 instead of bit 0. This patch fixes this issue Signed-off-by: Murali Karicherim-kariche...@ti.com Applying this for v3.5. But why not to 3.4? It concerns turning on DSP domain on DM644x. Yes, but I think as you noted sometime before, no one is really depending on kernel to turn on DSP domain on DM644x. Looks like you're right (though I don't remember noting it :-). Its probably being done from U-Boot or ROM code. This is not a v3.4 regression either. U-Boot does it only when DSPLINK support is not desired. Thanks, Sekhar WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH] arm: davinci: pdctl next bit position incorrect
Hello. On 19-04-2012 0:23, Sekhar Nori wrote: The PDCTL NEXT bit is incorrectly set to bit 1 instead of bit 0. This patch fixes this issue Signed-off-by: Murali Karicherim-kariche...@ti.com Applying this for v3.5. But why not to 3.4? It concerns turning on DSP domain on DM644x. WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH] davinci_emac: Add cpu_freq support
Hello.
On 09-04-2012 14:49, Manjunathappa, Prakash wrote:
Reconfigure interrupt coalesce parameter for changed emac bus_freq
due to DVFS.
Signed-off-by: Manjunathappa, Prakashprakash...@ti.com
---
drivers/net/ethernet/ti/davinci_emac.c | 60
1 files changed, 60 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ethernet/ti/davinci_emac.c
b/drivers/net/ethernet/ti/davinci_emac.c
index 174a334..11d3bd7 100644
--- a/drivers/net/ethernet/ti/davinci_emac.c
+++ b/drivers/net/ethernet/ti/davinci_emac.c
[...]
@@ -1761,6 +1765,46 @@ static const struct net_device_ops emac_netdev_ops = {
#endif
};
+#ifdef CONFIG_CPU_FREQ
+static int davinci_emac_cpufreq_transition(struct notifier_block *nb,
+unsigned long val, void *data)
+{
+ int ret = 0;
+ struct emac_priv *priv;
+
+ priv = container_of(nb, struct emac_priv, freq_transition);
+ if (priv-coal_intvl != 0) {
+ if (val == CPUFREQ_POSTCHANGE) {
+ if (emac_bus_frequency != clk_get_rate(emac_clk)) {
These 3 *if*s could be collapsed into one, and so indentation level
lowered significantly.
+ struct ethtool_coalesce coal;
+
+ emac_bus_frequency = clk_get_rate(emac_clk);
+
+ priv-bus_freq_mhz = (u32)(emac_bus_frequency /
+ 100);
+ coal.rx_coalesce_usecs = (priv-coal_intvl
+ 4);
+ ret = emac_set_coalesce(priv-ndev,coal);
+ }
+ }
+ }
+ return ret;
+}
+
+static inline int davinci_emac_cpufreq_register(struct emac_priv *priv)
+{
+ priv-freq_transition.notifier_call = davinci_emac_cpufreq_transition;
+ return cpufreq_register_notifier(priv-freq_transition,
+CPUFREQ_TRANSITION_NOTIFIER);
+}
+
+static inline void davinci_emac_cpufreq_deregister(struct emac_priv *priv)
+{
+ cpufreq_unregister_notifier(priv-freq_transition,
+ CPUFREQ_TRANSITION_NOTIFIER);
+}
+#endif
+
/**
* davinci_emac_probe: EMAC device probe
* @pdev: The DaVinci EMAC device that we are removing
@@ -1925,8 +1969,21 @@ static int __devinit davinci_emac_probe(struct
platform_device *pdev)
(regs: %p, irq: %d)\n,
(void *)priv-emac_base_phys, ndev-irq);
}
+
+#ifdef CONFIG_CPU_FREQ
+ rc = davinci_emac_cpufreq_register(priv);
+ if (rc) {
+ dev_err(pdev-dev, error in register_netdev\n);
Really?
+ rc = -ENODEV;
+ goto cpufreq_reg_err;
+ }
+#endif
return 0;
+#ifdef CONFIG_CPU_FREQ
+cpufreq_reg_err:
+ unregister_netdev(ndev);
+#endif
netdev_reg_err:
clk_disable(emac_clk);
no_irq_res:
@@ -1973,6 +2030,9 @@ static int __devexit davinci_emac_remove(struct
platform_device *pdev)
release_mem_region(res-start, resource_size(res));
+#ifdef CONFIG_CPU_FREQ
+ davinci_emac_cpufreq_deregister(priv);
+#endif
This is considered a bad practice to use #ifdef in the body of function.
Define the faunction you call here as empty inline in case CONFIG_CPU_FREQ is
not defined instead. The same about davinci_emac_cpufreq_register().
WBR, Sergei
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH] davinci: pdctl next bit position incorrect
Hello. On 04-04-2012 18:51, m-kariche...@ti.com wrote: From: Murali Karicherim-kariche...@ti.com The PDCTL NEXT bit is incorrectly set to bit 1 instead of bit 0. This patch fixes this issue Signed-off-by: Murali Karicherim-kariche...@ti.com diff --git a/arch/arm/mach-davinci/include/mach/psc.h b/arch/arm/mach-davinci/include/mach/psc.h index 39bf6ae..a03b8ba 100644 --- a/arch/arm/mach-davinci/include/mach/psc.h +++ b/arch/arm/mach-davinci/include/mach/psc.h @@ -298,7 +298,7 @@ #define MDSTAT_STATE_MASK 0x3f #define PDSTAT_STATE_MASK 0x1f #define MDCTL_FORCE BIT(31) -#define PDCTL_NEXT BIT(1) +#define PDCTL_NEXT BIT(0) And how did I miss this? :-/ WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 1/2] arm:da8xx:usb: move common OHCI platform setup code to mach-davinci/usb.c
Hello.
On 03/12/2012 05:05 PM, Manjunathappa, Prakash wrote:
Patch abstracts and moves common USB OHCI platform setup code of da8xx
based boards to mach-davinci/usb.c file, thereby it avoids repetition of
code.
Patch is based on consolidation concern raised by Sekhar Nori.
https://lkml.org/lkml/2011/12/21/48
Signed-off-by: Manjunathappa, Prakashprakash...@ti.com
---
arch/arm/mach-davinci/board-da830-evm.c | 86 +++-
arch/arm/mach-davinci/board-omapl138-hawk.c | 93 +++---
arch/arm/mach-davinci/include/mach/da8xx.h |2 +
arch/arm/mach-davinci/include/mach/usb.h| 32 +++-
arch/arm/mach-davinci/usb.c | 116 +++
drivers/usb/host/ohci-da8xx.c | 15 ++--
6 files changed, 178 insertions(+), 166 deletions(-)
diff --git a/arch/arm/mach-davinci/board-da830-evm.c
b/arch/arm/mach-davinci/board-da830-evm.c
index dc1afe5..edc8277 100644
--- a/arch/arm/mach-davinci/board-da830-evm.c
+++ b/arch/arm/mach-davinci/board-da830-evm.c
@@ -46,59 +46,23 @@ static const short da830_evm_usb11_pins[] = {
[...]
- /* TPS2065 switch @ 5V */
- .potpgt = (3 + 1) / 2, /* 3 ms max */
Why are you removing this initializer? You don't seem to pass it anywhere
else...
+ .type = GPIO_BASED,
+ .method = {
+ .gpio_method = {
+ .power_control_pin = ON_BD_USB_DRV,
+ .over_current_indicator = ON_BD_USB_OVC,
+ },
+ },
+ .board_ocic_handler = da830_evm_usb_ocic_irq,
};
diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c
b/arch/arm/mach-davinci/board-omapl138-hawk.c
index 45e8157..9214923 100644
--- a/arch/arm/mach-davinci/board-omapl138-hawk.c
+++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
@@ -184,77 +184,34 @@ mmc_setup_wp_fail:
[...]
static struct da8xx_ohci_root_hub omapl138_hawk_usb11_pdata = {
- .set_power = hawk_usb_set_power,
- .get_power = hawk_usb_get_power,
- .get_oci= hawk_usb_get_oci,
- .ocic_notify= hawk_usb_ocic_notify,
- /* TPS2087 switch @ 5V */
- .potpgt = (3 + 1) / 2, /* 3 ms max */
Same question.
diff --git a/arch/arm/mach-davinci/include/mach/usb.h
b/arch/arm/mach-davinci/include/mach/usb.h
index e0bc4ab..1a6f211 100644
--- a/arch/arm/mach-davinci/include/mach/usb.h
+++ b/arch/arm/mach-davinci/include/mach/usb.h
[...]
@@ -33,25 +35,47 @@
#define CFGCHIP2_REFFREQ_12MHZ(1 0)
#define CFGCHIP2_REFFREQ_24MHZ(2 0)
#define CFGCHIP2_REFFREQ_48MHZ(3 0)
Empty line needed here.
+enum usb_power_n_ovc_method {
+ GPIO_BASED = 1,
+};
structda8xx_ohci_root_hub;
typedef void (*da8xx_ocic_handler_t)(struct da8xx_ohci_root_hub *hub,
unsigned port);
+struct gpio_based {
+ u32 power_control_pin;
+ u32 over_current_indicator;
Aren't GPIOs just 'unsigned' in gpiolib, without size?
+};
+
/* Passed as the platform data to the OHCI driver */
structda8xx_ohci_root_hub {
/* Switch the port power on/off */
- int (*set_power)(unsigned port, int on);
+ int (*set_power)(unsigned port, struct da8xx_ohci_root_hub *hub,
+ int on);
/* Read the port power status */
- int (*get_power)(unsigned port);
+ int (*get_power)(unsigned port, struct da8xx_ohci_root_hub *hub);
/* Read the port over-current indicator */
- int (*get_oci)(unsigned port);
+ int (*get_oci)(unsigned port, struct da8xx_ohci_root_hub *hub);
/* Over-current indicator change notification (pass NULL to disable) */
- int (*ocic_notify)(da8xx_ocic_handler_t handler);
+ int (*ocic_notify)(da8xx_ocic_handler_t handler,
+ struct da8xx_ohci_root_hub *hub);
/* Time from power on to power good (in 2 ms units) */
u8 potpgt;
+
+ /* Power control and over current control method */
+ unsigned int type;
+ union power_n_overcurrent_pins {
+ struct gpio_based gpio_method;
+ /* Add data pertaining other methods here. For example if its
+* I2C based.
+*/
I thought that you need to first convert the existing GPIO-based method, and
think about the extensions when the need arises, in the next patches. You're
looking too far ahead in this patch...
+ } method;
+
+ /* board specific handler */
+ irq_handler_t board_ocic_handler;
};
void davinci_setup_usb(unsigned mA, unsigned potpgt_ms);
diff --git a/arch/arm/mach-davinci/usb.c b/arch/arm/mach-davinci/usb.c
index 23d2b6d..25c3520 100644
--- a/arch/arm/mach-davinci/usb.c
+++ b/arch/arm/mach-davinci/usb.c
@@ -11,12 +12,70 @@
#includemach/irqs.h
#includemach/cputype.h
#includemach/usb.h
Re: [PATCH] arm:da850: register musb device
On 03/13/2012 06:03 PM, Manjunathappa, Prakash wrote: Properly set up the OTG mode thru the CFGCHIP2 register, enable the USB0_DRVVBUS pin, I don't see where you are enabling it. and finally register the MUSB platform device. Signed-off-by: Ajay Kumar Guptaajay.gu...@ti.com Signed-off-by: Manjunathappa, Prakashprakash...@ti.com --- Patche are on top below posted patches: http://www.spinics.net/lists/linux-usb/msg59656.html arch/arm/mach-davinci/board-da850-evm.c | 12 arch/arm/mach-davinci/usb.c |9 + 2 files changed, 21 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c index 178d178..539e12c 100644 --- a/arch/arm/mach-davinci/board-da850-evm.c +++ b/arch/arm/mach-davinci/board-da850-evm.c @@ -787,6 +787,18 @@ static __init void da850_evm_usb_init(void) */ cfgchip2= ~CFGCHIP2_USB1PHYCLKMUX; cfgchip2 |= CFGCHIP2_USB2PHYCLKMUX; + /* +* We have to override VBUS/ID signals when MUSB is configured into the +* host-only mode -- ID pin will float if no cable is connected, so the +* controller won't be able to drive VBUS thinking that it's a B-device. +* Otherwise, we want to use the OTG mode and enable VBUS comparators. +*/ + cfgchip2 = ~CFGCHIP2_OTGMODE; +#if CONFIG_USB_MUSB_DA8XX This #ifdef doesn't make sense. It will make driver always work in host mode. + cfgchip2 |= CFGCHIP2_FORCE_HOST; +#else + cfgchip2 |= CFGCHIP2_SESENDEN | CFGCHIP2_VBDTCTEN; This is what you need to leave. The driver should always work in OTG mode now. +#endif __raw_writel(cfgchip2, DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG)); diff --git a/arch/arm/mach-davinci/usb.c b/arch/arm/mach-davinci/usb.c index 25c3520..a941dc5 100644 --- a/arch/arm/mach-davinci/usb.c +++ b/arch/arm/mach-davinci/usb.c @@ -284,6 +284,15 @@ void __init da8xx_board_usb_init(const short pins[], goto usb11_setup_fail; } + /* +* TPS2065 switch @ 5V supplies 1 A (sustains 1.5 A), +* with the power on to power good time of 3 ms. +*/ + ret = da8xx_register_usb20(1000, 3); + if (ret) + pr_warning(%s: USB 2.0 registration failed: %d\n, + __func__, ret); Er, why this code is moved there? And how will this work on DA830 -- try to register MUSB twice? Place this in the board file please where it belongs. WBR, Sergei ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
