Re: [PATCH] ARM: davinci: Remove redundant casts

[Sergei Shtylyov]

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 


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

[Sergei Shtylyov]

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 5/6] ARM: DTS: da850-evm: Add node for tlv320aic3106 codec

[Sergei Shtylyov]

Hello.

On 01-08-2014 9:22, Peter Ujfalusi wrote:


The board uses aic3106 for audio.



Signed-off-by: Peter Ujfalusi 
---
   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 2/6] ARM: DTS: da850: Add node for edma0

[Sergei Shtylyov]

On 07/31/2014 02:18 PM, Peter Ujfalusi wrote:


Add DT node for edma0.



Signed-off-by: Peter Ujfalusi 
---
  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 5/6] ARM: DTS: da850-evm: Add node for tlv320aic3106 codec

[Sergei Shtylyov]

Hello.

On 07/31/2014 02:18 PM, Peter Ujfalusi wrote:


The board uses aic3106 for audio.



Signed-off-by: Peter Ujfalusi 
---
  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 {


   The "reg" property is <0x18>, why the unit-address part of a name is 
different? 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.



+   #sound-dai-cells = <0>;
+   compatible = "ti,tlv320aic3106";
+   reg = <0x18>;
+   status = "okay";
+
+   /* Regulators */
+   IOVDD-supply = <&vdcdc2_reg>;
+   /* Derived from VBAT: Baseboard 3.3V / 1.8V */
+   AVDD-supply = <&vbat>;
+   DRVDD-supply = <&vbat>;
+   DVDD-supply = <&vbat>;
+   };
+


[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 v2 1/4] mdio_bus: implement devm_mdiobus_alloc/devm_mdiobus_free

[Sergei Shtylyov]

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 
Signed-off-by: Grygorii Strashko 

[...]


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 v2 1/4] mdio_bus: implement devm_mdiobus_alloc/devm_mdiobus_free

[Sergei Shtylyov]

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 
Signed-off-by: Grygorii Strashko 

[...]


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 2/2] net: davinci_mdio: use devm_* api

[Sergei Shtylyov]

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 
---
  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 1/2] mdio_bus: implement devm_mdiobus_alloc/devm_mdiobus_free

[Sergei Shtylyov]

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 

[...]


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 06/62] ARM: davinci: export da8xx_syscfg0_base

[Sergei Shtylyov]

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

[Sergei Shtylyov]

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

[Sergei Shtylyov]

Hello.

On 03/20/2014 02:50 PM, Arnd Bergmann wrote:


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.



I only care about it because I have the intention to get all 'randconfig'
kernels to build eventually. For stuff that is definitely 'legacy'
and won't get cleaned up properly, I'm fine with a hack.



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.



Yes, that would work, too. However, that would cause problems later
if we ever try to make the davinci platform "multiplatform", unless we also
pass the physical address in a platform resource and make this a larger



Some SATA driver work done by Bartlomiej Zolnierkiewicz needed driver
access to syscfg registers too and I just asked him to pass the physical
addresses using platform resource. I think thats the best bet we have in
the absence of a modern interface.



Ok.


   Depends on what SYSCFG register it uses. It wouldn't be good if that 
register range includes CFGCHIP2.



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.



CFGCHIP2 has controls both for USB 1.1 (OHCI) and USB 2.0 (MUSB). Not
sure if there can be a single PHY driver for both.



The phy infrastructure explicitly supports multiple consumers for one
phy, if I'm reading the code correctly.


   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.



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

[Sergei Shtylyov]

Hello.

On 20-03-2014 10:42, Arnd Bergmann wrote:


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.


   ... also it was suggested to pass the CFGCHIP2 address as a resource. I 
certainly wasn't eager to do that since if done for both MUSB and OHCI, it 
would cause sort of a resource conflict (platform device resources are 
automatically registered in the resource tree, although without marking them 
exclusive), even if we don't call request_mem_region() on them (we can't do 
that since in this case the resource would be registered as exclusive, and the 
second call would fail).



Yes, that would work, too. However, that would cause problems later
if we ever try to make the davinci platform "multiplatform", unless we also
pass the physical address in a platform resource and make this a larger
driver.


   In my opinion, we don't have to pass CFGCHIP2 as a resource and could just 
ioremap() the raw physical address.



I think we can reasonably have an exported set of functions
declared in the platform data header file for these drivers, but passing
the physical address through a header file


   That's how it's passed today (however, thru ).


wouldn't be much of an improvement
over passing the virtual address.


   Not sure I understood about passing virtual address.


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.



Good point.



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).



I think they are also required for 'randconfig' builds to some degree, because
you can build a kernel without host mode for instance.


   Yes.


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.



Yes, a phy driver seems the right approach if we can find someone to do it.


   Perhaps a person that tried to unbreak the DA8xx MUSB driver could be 
interested (and competent) enough to do it...



Ideally that should let us use generic versions of the ohci and musb drivers
even, if that's the only davinci specific part of these drivers.


   No, not really. MUSB ceratinly has DaVinci specific register set mapped in 
its register space. OHCI has up to 2 clocks to enable since USB 1.1 PHY can be 
clocked from USB 2.0 PHY clock.



based on a syscon driver.



 I don't know what syscon is...



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?


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.



For USB 2.0 compliance you actually need to provide something to handle the
low speed modes. A lot of people use a hub to do that nowadays rather than
an OHCI or UHCI, but I don't know if that works with OTG.


   MUSB handles all speeds. I think the reason TI also included USB 1.1 
controller lies in the somewhat dubious reputation of both MUSB hardware and 
the Linux driver.



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

[Sergei Shtylyov]

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 
Cc: Sekhar Nori 
Cc: Kevin Hilman 
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 06/62] ARM: davinci: export da8xx_syscfg0_base

[Sergei Shtylyov]

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 
Cc: Sekhar Nori 
Cc: Kevin Hilman 
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 0/5] ARM: davinci: tnetv107x removal

[Sergei Shtylyov]

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

[Sergei Shtylyov]

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

Re: [PATCH 2/2] DT: net: davinci_emac: "ti, davinci-no-bd-ram" property is actually optional

[Sergei Shtylyov]

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 



---
  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: 

   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 2/2] DT: net: davinci_emac: "ti, davinci-no-bd-ram" property is actually optional

[Sergei Shtylyov]

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 

---
 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: 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

[Sergei Shtylyov]

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 

---
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: http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

[PATCH 0/2] DT: net: davinci_emac: couple more properties actually optional

[Sergei Shtylyov]

Hello.

   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

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

[Sergei Shtylyov]

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 

---
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: 
 
 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()

[Sergei Shtylyov]

Hello.

On 27-08-2013 11:05, Vishwanathrao Badarkhe, Manish wrote:


From: "Vishwanathrao Badarkhe, Manish" 



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 
---
: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

[Sergei Shtylyov]

Hello.

On 08/16/2013 06:11 PM, Lad, Prabhakar wrote:


From: "Lad, Prabhakar" 



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 
---
  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 5/5] ARM: davinci: da850: add OF_DEV_AUXDATA entry for eth0.

[Sergei Shtylyov]

Hello.

On 15-08-2013 10:01, Lad, Prabhakar wrote:


From: "Lad, Prabhakar" 



Add OF_DEV_AUXDATA for eth0  driver in da850 board dt
file to use emac clock.



Signed-off-by: Lad, Prabhakar 

[...]


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 v4 4/5] ARM: davinci: da850: add DT node for eth0.

[Sergei Shtylyov]

Hello.

On 15-08-2013 10:01, Lad, Prabhakar wrote:


From: "Lad, Prabhakar" 



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 

[...]


@@ -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 v3 4/9] media: davinci: vpif_capture: move the freeing of irq and global variables to remove()

[Sergei Shtylyov]

Hello.

On 26-05-2013 16:00, Prabhakar Lad wrote:


From: Lad, Prabhakar 



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 
---
  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 v3 2/9] media: davinci: vpif: Convert to devm_* api

[Sergei Shtylyov]

Hello.

On 26-05-2013 16:00, Prabhakar Lad wrote:


From: Lad, Prabhakar 



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 
Acked-by: Laurent Pinchart 
---
  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 v2 2/5] media: davinci: vpif: Convert to devm_* api

[Sergei Shtylyov]

Hello.

On 26-05-2013 4:49, Laurent Pinchart wrote:


From: Lad, Prabhakar 



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 
---
  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..164c1b7 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);
-   if (!res)
-   return -ENOENT;
-
-   res_len = resource_size(res);
-
-   res = request_mem_region(res->start, res_len, res->name);
-   if (!res)
-   return -EBUSY;
-
-   vpif_base = ioremap(res->start, res_len);
-   if (!vpif_base) {
-   status = -EBUSY;
-   goto fail;
-   }
+   vpif_base = devm_ioremap_resource(&pdev->dev, res);
+   if (IS_ERR(vpif_base))
+   return PTR_ERR(vpif_base);



You're loosing the request_mem_region().


   He's not losing anything, first look at how devm_ioremp_resource() 
is defined.



You should use devm_request_and_ioremap()


Already deprecated by now.


function instead of devm_ioremap_resource(). With
that change,



Acked-by: Laurent Pinchart 


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

[Sergei Shtylyov]

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 
---
  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 5/5] net: davinci_mdio: trivial cleanup

[Sergei Shtylyov]

On 16-05-2013 11:30, Lad Prabhakar wrote:


From: Lad, Prabhakar 



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 


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

[Sergei Shtylyov]

On 16-05-2013 11:30, Lad Prabhakar wrote:


From: Lad, Prabhakar 



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 


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

[Sergei Shtylyov]

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 


   Changelog won't hurt here... which unwanted #include's you are 
removing and why are they unwanted?



Signed-off-by: Lad, Prabhakar 
---
  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 
-#include 
-#include 
+
+#include 
+#include 
  #include 
  #include 
-#include 
-#include 
-#include 
-#include 

  #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] ARM: davinci: dma: Convert to devm_* api

[Sergei Shtylyov]

Hello.

On 16-05-2013 10:58, Lad Prabhakar wrote:


From: Lad, Prabhakar 



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 
---
  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] media: davinci: vpbe: fix layer availability for NV12 format

[Sergei Shtylyov]

Hello.

On 03-05-2013 13:53, Prabhakar Lad wrote:


From: Lad, Prabhakar 



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 
---
  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 v2] media: davinci: vpif: align the buffers size to page page size boundary

[Sergei Shtylyov]

Hello.

On 16-04-2013 14:54, Prabhakar lad wrote:


From: Lad, Prabhakar 



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 
Cc: Laurent Pinchart 
Cc: Hans Verkuil 
Cc: Mauro Carvalho Chehab 


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

[Sergei Shtylyov]

On 16-04-2013 15:10, Prabhakar lad wrote:


From: Lad, Prabhakar 



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 
Cc: Laurent Pinchart 
Cc: Hans Verkuil 
Cc: Mauro Carvalho Chehab 


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

[Sergei Shtylyov]

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

[Sergei Shtylyov]

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

[Sergei Shtylyov]

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 
---
  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

[Sergei Shtylyov]

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 (

Re: DM6446 using multiple USB devices

[Sergei Shtylyov]

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: DM6446 using multiple USB devices

[Sergei Shtylyov]

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  hub.



We are trying to hook up 2 usbnet devices (using the cdc_ethernet driver).



Each device needs the following EPs.



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: [PATCH] watchdog: davinci_wdt: update to devm_* API

[Sergei Shtylyov]

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 
---
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

[Sergei Shtylyov]

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 
---
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 v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Sergei Shtylyov]

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

[Sergei Shtylyov]

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

[Sergei Shtylyov]

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 2/2] ARM: davinci: da850: add wdt OF_DEV_AUXDATA entry

[Sergei Shtylyov]

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" 
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 
Signed-off-by: Sekhar Nori 

[...]


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

[Sergei Shtylyov]

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

[Sergei Shtylyov]

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

[Sergei Shtylyov]

Hello.

On 02-02-2013 16:45, Russell King - ARM Linux wrote:


Now, CPPI is brand new code to arch/arm - always has been.  It post-dates
the DMA engine API.  And it's been said many times about moving it to
drivers/dma.  The problem is Sergei doesn't want to do it - he's anti the


   I *can't* do it, and I have denied all further responibility for it.


DMA engine API for whatever reasons he can dream up.  He professes no


   I'm not dreaming anything up. Please understand that CPPI 4.1 is not just 
a normal DMA controller -- it's a complex of several devices with DMA 
controller being only one of them, and one that can't work autonomously but 
only thru the proxy of the queue manager. That queue manager stuff I found 
hard to fit into drivers/dma/ infrastructure. Myabe it was a honest mistake, 
of course.



knowledge of my dislike for having it in arch/arm, yet there's emails
from years back showing that he knew about it.  TI knows about it; Ajay
about it.  Yet... well... history speaks volumes about this.


   Some details have slipped rom my memory after that many years. Im sorry 
for that.



Now, there may be a new problem with CPPI.  That being we're now requiring
DT support.  _Had_ this been done prior to the push for DT, then it would
not have been a concern, because then the problem becomes one for DT.
But now that OMAP is being converted to DT, and DT is The Way To Go now,
that's an additional problem that needs to be grappled with - or it may
make things easier.


   DaVinci is also being converted to device tree. However, that support 
remains optional for now. Not sure it will make things easier, as we still 
have two distinct devices to declare in the device tree: DMA controller and 
queue manager, and we'll have to describe the interconnect between them too 
(as a props of DMA controller I guess)...


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

[Sergei Shtylyov]

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

[Sergei Shtylyov]

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  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 about put

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Sergei Shtylyov]

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 8t

Re: [PATCH v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Sergei Shtylyov]

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 v7 01/10] ARM: davinci: move private EDMA API to arm/common

[Sergei Shtylyov]

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

[Sergei Shtylyov]

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

[Sergei Shtylyov]

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

[Sergei Shtylyov]

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

[Sergei Shtylyov]

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

[Sergei Shtylyov]

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 v6 1/2] ARM: davinci: Remoteproc driver support for OMAP-L138 DSP

[Sergei Shtylyov]

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 v6 2/2] ARM: davinci: Remoteproc platform device creation data/code

[Sergei Shtylyov]

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

[Sergei Shtylyov]

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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include/* 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


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 firmware name then
+* construct a default name.  We're already glomming 'len'
+* bytes onto the end of the struct rproc al

Re: [PATCH v2] davinci_nand: fix modular build with CONFIG_OF=y

[Sergei Shtylyov]

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 
> 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

[PATCH v2] davinci_nand: fix modular build with CONFIG_OF=y

[Sergei Shtylyov]

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 
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

[Sergei Shtylyov]

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

Re: [PATCH v6] ARM: mtd: nand: davinci: add OF support for davinci nand controller

[Sergei Shtylyov]

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

[Sergei Shtylyov]

Hello.

On 03-01-2013 11:03, Heiko Schocher wrote:


add OF support for the davinci nand controller.



Signed-off-by: Heiko Schocher
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
Cc: Grant Likely
Cc: Sekhar Nori
Cc: Wolfgang Denk
Cc: Scott Wood



[...]



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: Are TI DM8168/DM8148 considered DaVinci processors ?

[Sergei Shtylyov]

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

[PATCH] davinci_nand: fix compilation with CONFIG_OF=y

[Sergei Shtylyov]

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 
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 v6] ARM: mtd: nand: davinci: add OF support for davinci nand controller

[Sergei Shtylyov]

Hello.

On 07/30/2012 11:22 AM, Heiko Schocher wrote:

> add OF support for the davinci nand controller.

> Signed-off-by: Heiko Schocher 
> 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 
> Cc: Grant Likely 
> Cc: Sekhar Nori 
> Cc: Wolfgang Denk 
> Cc: Scott Wood 

[...]

> 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

Re: [PATCH v3 2/6] ARM: davinci: Changed pr_warning() to pr_warn() (part 2)

[Sergei Shtylyov]

Hello.

On 15-11-2012 5:53, Tivy, Robert wrote:


Also, while modifying those pr_warning() calls I changed hardcoded
function names to use '"%s:", __func__' instead



Signed-off-by: Robert Tivy 
---
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


   I meant "talking". :-/


shouldn't have been separated from patch 1 at all -- since it's two
boards built around the same chip, OMAP-L138...



The 4 patches that are of the same nature ("Changed pr_warning() to pr_warn() (part 
#)") were split as 4 separate patches on request by Sekhar, for the purpose of 
making it easier to merge later.


   To quote Sekhar: "My main motivation was to keep board and soc file 
changes from mixing together." You just went too far.


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

[Sergei Shtylyov]

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

Re: [PATCH v3 2/6] ARM: davinci: Changed pr_warning() to pr_warn() (part 2)

[Sergei Shtylyov]

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 
---
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: [PATCH v3 1/6] ARM: davinci: Changed pr_warning() to pr_warn() (part 1)

[Sergei Shtylyov]

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 
---
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);

   

State of DM6446 EVM in mainline

[Sergei Shtylyov]

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()

[Sergei Shtylyov]

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 
---
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

[Sergei Shtylyov]

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 
Signed-off-by: Manjunath Hadli 
Cc: Sekhar Nori 
Cc: Sergei Shtylyov 



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

[Sergei Shtylyov]

Hello.

On 22-10-2012 16:12, Prabhakar Lad wrote:


From: Lad, Prabhakar 



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 
Signed-off-by: Manjunath Hadli 
Cc: Sekhar Nori 
---
  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

[Sergei Shtylyov]

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 



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

[Sergei Shtylyov]

Hello.

On 10-10-2012 14:33, Constantine Shulyupin wrote:


From: Constantine Shulyupin 

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 
---

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-kernel&m=130894150803661&w=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 
  #include 
  #include 
+#include 
  
  #include 
  
@@ -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

[Sergei Shtylyov]

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 



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

[Sergei Shtylyov]

Hello.

On 09/14/2012 05:53 PM, Prabhakar Lad wrote:

> From: Lad, Prabhakar 

> 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 
> Signed-off-by: Lad, Prabhakar 
> Signed-off-by: Manjunath Hadli 
> Cc: Hans Verkuil 

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

[Sergei Shtylyov]

Hello.

On 08/25/2012 09:41 PM, Julia Lawall wrote:

> From: Julia Lawall 

> 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 

> ---
> 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

[Sergei Shtylyov]

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 
Cc: Anatolij Gustschin 

[...]


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 0/1] media/video: vpif: fixing function name start to vpif_config_params

[Sergei Shtylyov]

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 1/1] media/video: vpif: fixing function name start to vpif_config_params

[Sergei Shtylyov]

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 v2 6/6] arm/dts: am33xx rtc node

[Sergei Shtylyov]

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 v2 6/6] arm/dts: am33xx rtc node

[Sergei Shtylyov]

Hello.

On 25-07-2012 10:12, Afzal Mohammed wrote:


Add AM33xx rtc node.



Signed-off-by: Afzal Mohammed 
---



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 1/6] rtc: omap: kicker mechanism support

[Sergei Shtylyov]

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 
---



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   (1<<7)
  #define OMAP_RTC_CTRL_DISABLE (1<<6)
@@ -88,11 +93,19 @@
  #define OMAP_RTC_INTERRUPTS_IT_ALARM(1<<3)
  #define OMAP_RTC_INTERRUPTS_IT_TIMER(1<<2)

+/* 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] da8xx-fb: enable sync lost interrupt

[Sergei Shtylyov]

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 
---
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 2/6] ARM: davinci: remove rtc kicker release

[Sergei Shtylyov]

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 
---
  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 RESEND] video: da8xx-fb: enable sync lost interrupt

[Sergei Shtylyov]

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 
> ---
> 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

[Sergei Shtylyov]

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 
Signed-off-by: Manjunathappa, Prakash 
---
  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

[Sergei Shtylyov]

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 Hadli
Signed-off-by: Lad, Prabhakar
---
  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

[Sergei Shtylyov]

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 Povey
Cc: Sekhar Nori
Cc: Felipe Balbi
---
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

[Sergei Shtylyov]

On 05/22/2012 05:55 PM, Heiko Schocher wrote:


Signed-off-by: Heiko Schocher
Cc: davinci-linux-open-source@linux.davincidsp.com
Cc: linux-arm-ker...@lists.infradead.org
Cc: devicetree-disc...@lists.ozlabs.org
Cc: Grant Likely
Cc: Sekhar Nori
Cc: Wolfgang Denk
Cc: Sergei Shtylyov


   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

[Sergei Shtylyov]

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