Re: [PATCH 0/5] AM33xx: MMC resources from DT without HWMOD data

2013-06-27 Thread Joel A Fernandes 
Hi Benoit,

On Thu, Jun 27, 2013 at 7:03 AM, Benoit Cousson b-cous...@ti.com wrote:
 Hi Joel,

 On 06/26/2013 05:28 AM, Joel A Fernandes wrote:
 On Tue, Jun 25, 2013 at 8:23 PM, Joel A Fernandes joelag...@ti.com wrote:
 From: Joel A Fernandes agnel.j...@gmail.com

 This series is fixes to get MMC working on AM33XX without HWMOD data.
 On removal of HWMOD data, interrupt and register properties need to be 
 provided
 for the driver to function correctly.

 Please don't pull patches authored by me in this series. I have posted
 them with my wrong email address by mistake. I will correct my email
 during the next repost, after a round of review. Matt's patches in the
 series are still good though.

 In that case, please repost these in two series to avoid pulling the DTS
 files in the MMC tree.

The series has been replaced with the single patch:
[PATCH v2] ARM: dts: add AM33XX MMC support
http://marc.info/?l=linux-omapm=137230403815943w=2

Some patches were squashed and others dropped in the series resulting
in the single
patch above. This patch should be good to apply

Thanks,

-Joel
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 0/2] EDMA config and comments

2013-06-25 Thread Joel A Fernandes 
On Tuesday, June 25, 2013, Joel A Fernandes wrote:

 From: Joel A Fernandes agnel.j...@gmail.com javascript:;


Please donot pull these patches. I have posted them with my wrong email
address by mistake. I will correct my email during the next repost,
after a round
of review.

-Joel


 Minor patches remaining after EDMA series has been merged:

 First patch adds the TI_EDMA option which people can forget to add and
 can result in failure without any informative errors of why DMA is not
 working.

 Second patch adds comments to the EDMA code for the calculations in the
 A-sync case. From my experience, this code is not-so-obvious. Hopefully
 this makes the life of readers of the code a little better.

 Joel A Fernandes (2):
   ARM: configs: Enable TI_EDMA in omap2plus_defconfig
   DMA: EDMA: Add comments for A-sync case calculations

  arch/arm/configs/omap2plus_defconfig |1 +
  drivers/dma/edma.c   |   22 ++
  2 files changed, 23 insertions(+)

 --
 Resending as first attempt didn't work.

 1.7.9.5

 --
 To unsubscribe from this list: send the line unsubscribe linux-omap in
 the body of a message to majord...@vger.kernel.org javascript:;
 More majordomo info at  http://vger.kernel.org/majordomo-info.html

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 0/5] AM33xx: MMC resources from DT without HWMOD data

2013-06-25 Thread Joel A Fernandes 
On Tue, Jun 25, 2013 at 8:23 PM, Joel A Fernandes joelag...@ti.com wrote:
 From: Joel A Fernandes agnel.j...@gmail.com

 This series is fixes to get MMC working on AM33XX without HWMOD data.
 On removal of HWMOD data, interrupt and register properties need to be 
 provided
 for the driver to function correctly.

Please don't pull patches authored by me in this series. I have posted
them with my wrong email address by mistake. I will correct my email
during the next repost, after a round of review. Matt's patches in the
series are still good though.

Thanks,

-Joel
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH v12 05/11] edma: config: Enable config options for EDMA

2013-06-24 Thread Joel A Fernandes 
On Mon, Jun 24, 2013 at 6:53 AM, Sekhar Nori nsek...@ti.com wrote:
 On 6/22/2013 8:23 AM, Joel A Fernandes wrote:
 config TI_EDMA
 tristate TI EDMA support
 default m if 'ARCH_DAVINCI || ARCH_OMAP1 || ARCH_OMAP2
 select DMA_ENGINE
 select DMA_VIRTUAL_CHANNELS


 MMC depends on EDMA specially on AM33xx there's no PIO mode AFAIK. The
 'm' option will require some initramfs to load the module when needing
 to MMC boot, I suggest lets leave it as y.


 Ah, right: you still export a filter function from the edma driver
 and use it in slave drivers:

 drivers/mmc/host/davinci_mmc.c: 
 dma_request_slave_channel_compat(mask, edma_filter_fn,
 drivers/mmc/host/davinci_mmc.c: 
 dma_request_slave_channel_compat(mask, edma_filter_fn,
 drivers/spi/spi-davinci.c:  dspi-dma_rx = dma_request_channel(mask, 
 edma_filter_fn,
 drivers/spi/spi-davinci.c:  dspi-dma_tx = dma_request_channel(mask, 
 edma_filter_fn,

 As long as this is the case, you have to be careful with the dependencies
 to make sure that davinci_mmc and spi-davinci either depend on TI_EDMA, or
 edma_filter_fn gets defined to NULL when you are building for a DT-only 
 platform.

 Yes sure, right now they are defined  as follows in include/linux/edma.h:

 #if defined(CONFIG_TI_EDMA) || defined(CONFIG_TI_EDMA_MODULE)
 bool edma_filter_fn(struct dma_chan *, void *);
 #else
 static inline bool edma_filter_fn(struct dma_chan *chan, void *param)
 {
 return false;
 }
 #endif

 This also has the side effect of causing DMA requests to fail if
 TI_EDMA is not built, causing frustration for a lot of people some of
 whom don't want to deal with DMA so I think it is OK to build the
 driver in by default as it is (and will be) used by a lot of
 OMAP2PLUS.

 Solution for this is to enable TI_EDMA in relevant defconfigs. Most
 folks who would get frustrated by such issues would be using defconfigs
 and for those who are building their configuration from scratch this
 will be pretty low in their list of worries.

Yes, it is not in omap2plus_defconfig either. I will post a patch to
add it to the same, and we can ignore the Kconfig defaults and select
patches.

Thanks,
Joel
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH v12 05/11] edma: config: Enable config options for EDMA

2013-06-24 Thread Joel A Fernandes 
On Mon, Jun 24, 2013 at 6:23 AM, Sekhar Nori nsek...@ti.com wrote:
 On 6/22/2013 3:23 AM, Joel A Fernandes wrote:
 Hi Arnd,

 On Fri, Jun 21, 2013 at 1:44 PM, Arnd Bergmann a...@arndb.de wrote:
 On Friday 21 June 2013, Joel A Fernandes wrote:
 I think we are talking about different things, I agree the 'select
 DMADEVICES' can be dropped but lets please keep the default y option
 (not adding new select statements, just saying that if someone select
 DMADEVICES in menuconfig and if they're ARCH_OMAP=1 , then default to
 EDMA). This will simply allow people to have a default. Thanks.

 Yes, that's ok.

 Ok, thanks. I will follow up with a patch in my next submissions that builds 
 it.

 Perhaps a:
 default y if 'ARCH_OMAP2PLUS'

 and leave the existing as it is...
 default n if  'ARCH_DAVINCI || ARCH_OMAP1 || ARCH_OMAP2'

 would make most sense to me. Basically EDMA is seen on current and all
 new OMAP2PLUS.

 OMAP2PLUS devices like OMAP3/4 do not have EDMA so this is not really true.

Sure. Right now though, and from what I've seen, all future SoCs are
supporting EDMA.



 config TI_EDMA
 tristate TI EDMA support
 default m if 'ARCH_DAVINCI || ARCH_OMAP1 || ARCH_OMAP2
 select DMA_ENGINE
 select DMA_VIRTUAL_CHANNELS



 MMC depends on EDMA specially on AM33xx there's no PIO mode AFAIK. The
 'm' option will require some initramfs to load the module when needing
 to MMC boot, I suggest lets leave it as y.

 But there is no reason why it cannot work with PIO, right? Sounds like
 the right fix is in driver.

I am not sure about this. I agree no reason it cannot do PIO. I will check.
But even if it did, loading EDMA as a module will require omap_hsmmc to switch
to DMA mode which I am sure the driver doesn't support today.

Thanks,
Joel
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH v12 05/11] edma: config: Enable config options for EDMA

2013-06-24 Thread Joel A Fernandes 
On Mon, Jun 24, 2013 at 3:28 PM, Arnd Bergmann a...@arndb.de wrote:
 On Saturday 22 June 2013, Joel A Fernandes wrote:

   config TI_EDMA
   tristate TI EDMA support
   default m if 'ARCH_DAVINCI || ARCH_OMAP1 || ARCH_OMAP2
   select DMA_ENGINE
   select DMA_VIRTUAL_CHANNELS
 
 
  MMC depends on EDMA specially on AM33xx there's no PIO mode AFAIK. The
  'm' option will require some initramfs to load the module when needing
  to MMC boot, I suggest lets leave it as y.
 
 
  Ah, right: you still export a filter function from the edma driver
  and use it in slave drivers:
 
  drivers/mmc/host/davinci_mmc.c: 
  dma_request_slave_channel_compat(mask, edma_filter_fn,
  drivers/mmc/host/davinci_mmc.c: 
  dma_request_slave_channel_compat(mask, edma_filter_fn,
  drivers/spi/spi-davinci.c:  dspi-dma_rx = dma_request_channel(mask, 
  edma_filter_fn,
  drivers/spi/spi-davinci.c:  dspi-dma_tx = dma_request_channel(mask, 
  edma_filter_fn,
 
  As long as this is the case, you have to be careful with the dependencies
  to make sure that davinci_mmc and spi-davinci either depend on TI_EDMA, or
  edma_filter_fn gets defined to NULL when you are building for a DT-only 
  platform.

 Yes sure, right now they are defined  as follows in include/linux/edma.h:

 #if defined(CONFIG_TI_EDMA) || defined(CONFIG_TI_EDMA_MODULE)
 bool edma_filter_fn(struct dma_chan *, void *);
 #else
 static inline bool edma_filter_fn(struct dma_chan *chan, void *param)
 {
 return false;
 }
 #endif

 It's best to just define the filter function in the platform
 code and pass a pointer to it through platform data. The way you do
 it above makes the slave drivers inherently nonportable.

But with DT-only platforms, can you really do that?

Thanks,
Joel
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH v12 02/11] ARM: edma: Add DT and runtime PM support to the private EDMA API

2013-06-21 Thread Joel A Fernandes 
On Fri, Jun 21, 2013 at 4:37 AM, Sekhar Nori nsek...@ti.com wrote:
 Joel,

 Looks like you have not addressed all comments from last time.
 For now, in the interest of time, I have fixed them myself. But next
 time on, if you are not going to fix any comment, then please reply
 upfront before sending out *any* new versions.

Will do. I thought there was just 1 other disagreement. Neverthless
I will make it a point to reply upfront next time.

 Also, *please* run checkpatch and fix all build warnings before you send
 the patch out. You seem to have introduced new checkpatch errors and
 compile warnings with this version.

Ok, this really sucks. Somehow I missed the new checkpatch error.
Thanks for fixing it  Sorry.

Joel

 On 6/21/2013 2:36 AM, Joel A Fernandes wrote:
 From: Matt Porter mpor...@ti.com

 Adds support for parsing the TI EDMA DT data into the required EDMA
 private API platform data. Enables runtime PM support to initialize
 the EDMA hwmod. Enables build on OMAP.

 Changes by Joel:
 * Setup default one-to-one mapping for queue_priority and queue_tc
 mapping as discussed in [1].
 * Split out xbar stuff to separate patch. [1]
 * Dropped unused DT helper to convert to array

 [1] https://patchwork.kernel.org/patch/2226761/

 Signed-off-by: Matt Porter mpor...@ti.com
 Acked-by: Sekhar Nori nsek...@ti.com
 Signed-off-by: Joel A Fernandes joelag...@ti.com
 ---
  arch/arm/common/edma.c |  180 
 +---
  include/linux/platform_data/edma.h |4 +-
  2 files changed, 169 insertions(+), 15 deletions(-)

 diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c
 index 7658874..407e01e 100644
 --- a/arch/arm/common/edma.c
 +++ b/arch/arm/common/edma.c
 @@ -25,6 +25,13 @@
  #include linux/platform_device.h
  #include linux/io.h
  #include linux/slab.h
 +#include linux/edma.h
 +#include linux/err.h
 +#include linux/of_address.h
 +#include linux/of_device.h
 +#include linux/of_dma.h
 +#include linux/of_irq.h
 +#include linux/pm_runtime.h

  #include linux/platform_data/edma.h

 @@ -1369,13 +1376,102 @@ void edma_clear_event(unsigned channel)
  }
  EXPORT_SYMBOL(edma_clear_event);

 -/*---*/
 +static int edma_of_read_u32_to_s16_array(const struct device_node *np,
 +  const char *propname, s16 *out_values,
 +  size_t sz)
 +{
 + int ret;
 +
 + ret = of_property_read_u16_array(np, propname, out_values, sz);
 + if (ret)
 + return ret;
 +
 + /* Terminate it */
 + *out_values++ = -1;
 + *out_values++ = -1;
 +
 + return 0;
 +}

 I asked you to get rid of unused functions last time around. This
 function is unused here.

 +
 +static int edma_of_parse_dt(struct device *dev,
 + struct device_node *node,
 + struct edma_soc_info *pdata)
 +{
 + int ret = 0, i;
 + u32 value;
 + struct property *prop;
 + size_t sz;

 sz and prop are not used in this function and result in unused variable
 warnings.

 + struct edma_rsv_info *rsv_info;
 + s8 (*queue_tc_map)[2], (*queue_priority_map)[2];
 +
 + memset(pdata, 0, sizeof(struct edma_soc_info));
 +
 + ret = of_property_read_u32(node, dma-channels, value);
 + if (ret  0)
 + return ret;
 + pdata-n_channel = value;
 +
 + ret = of_property_read_u32(node, ti,edma-regions, value);
 + if (ret  0)
 + return ret;
 + pdata-n_region = value;
 +
 + ret = of_property_read_u32(node, ti,edma-slots, value);
 + if (ret  0)
 + return ret;
 + pdata-n_slot = value;
 +
 + pdata-n_cc = 1;
 + pdata-n_tc = 3;

 n_tc setting is not used in driver as I mentioned last time. I dropped
 this line.

 +
 + rsv_info =
 + devm_kzalloc(dev, sizeof(struct edma_rsv_info), GFP_KERNEL);
 + if (!rsv_info)
 + return -ENOMEM;
 + pdata-rsv = rsv_info;
 +
 + queue_tc_map = devm_kzalloc(dev, 8*sizeof(s8), GFP_KERNEL);
 + if (!queue_tc_map)
 + return -ENOMEM;
 +
 + for (i = 0; i  3; i++) {
 + queue_tc_map[i][0] = i;
 + queue_tc_map[i][1] = i;
 + }
 + queue_tc_map[i][0] = -1;
 + queue_tc_map[i][1] = -1;
 +
 + pdata-queue_tc_mapping = queue_tc_map;
 +
 + queue_priority_map = devm_kzalloc(dev, 8*sizeof(s8), GFP_KERNEL);
 + if (!queue_priority_map)
 + return -ENOMEM;
 +
 + for (i = 0; i  3; i++) {
 + queue_priority_map[i][0] = i;
 + queue_priority_map[i][1] = i;
 + }
 + queue_priority_map[i][0] = -1;
 + queue_priority_map[i][1] = -1;
 +
 + pdata-queue_priority_mapping = queue_priority_map;
 +
 + pdata-default_queue = 0;
 +
 + return ret;
 +}
 +
 +static struct of_dma_filter_info edma_filter_info = {
 + .filter_fn = edma_filter_fn,
 +};

 -static int __init edma_probe(struct

Re: [PATCH v12 05/11] edma: config: Enable config options for EDMA

2013-06-21 Thread Joel A Fernandes 
On Fri, Jun 21, 2013 at 5:16 AM, Sekhar Nori nsek...@ti.com wrote:
 On 6/21/2013 2:36 AM, Joel A Fernandes wrote:
 From: Joel A Fernandes agnel.j...@gmail.com

 Build TI_EDMA by default for ARCH_DAVINCI and ARCH_OMAP2PLUS

 Signed-off-by: Joel A Fernandes joelag...@ti.com

 You should sign-off with author e-mail address.

 ---
  arch/arm/Kconfig|1 +
  arch/arm/mach-omap2/Kconfig |1 +
  drivers/dma/Kconfig |2 +-
  3 files changed, 3 insertions(+), 1 deletion(-)

 diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
 index b1c66a4..7d58cd9 100644
 --- a/arch/arm/Kconfig
 +++ b/arch/arm/Kconfig
 @@ -841,6 +841,7 @@ config ARCH_DAVINCI
   select HAVE_IDE
   select NEED_MACH_GPIO_H
   select TI_PRIV_EDMA
 + select DMADEVICES

 It is generally a bad idea to force select on something that can be
 enabled using menuconfig. Unless used carefully, select causes unmet
 direct dependency warnings which folks are already fighting hard to
 fix. This leads to what Russell referred in the past as select madness [1]

Are you concerned with bloat issues? I know your point of view but the idea
was to build these options by default for these platforms even though
in some cases
it might not be used. I have seen folks including myself select the wrong
option. Having the build system automatically select the correct option for the
most common cases can be very useful I feel and not require manual
configuration.

 In this particular case, it is perfectly okay to have a DaVinci platform
 without DMA engine support. Drivers figure out they don't have DMA
 support and switch to PIO mode.

Drivers can still use PIO mode if they wish even though DMA is built into the
kernel right.

   select USE_OF
   select ZONE_DMA
   help
 diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
 index f91b07f..c02f083 100644
 --- a/arch/arm/mach-omap2/Kconfig
 +++ b/arch/arm/mach-omap2/Kconfig
 @@ -17,6 +17,7 @@ config ARCH_OMAP2PLUS
   select PROC_DEVICETREE if PROC_FS
   select SOC_BUS
   select SPARSE_IRQ
 + select DMADEVICES
   select TI_PRIV_EDMA
   select USE_OF
   help
 diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
 index 3215a3c..b2d6f15 100644
 --- a/drivers/dma/Kconfig
 +++ b/drivers/dma/Kconfig
 @@ -216,7 +216,7 @@ config TI_EDMA
   depends on ARCH_DAVINCI || ARCH_OMAP
   select DMA_ENGINE
   select DMA_VIRTUAL_CHANNELS
 - default n
 + default y

 Can't see why DMA support should default to y.


For same reason as above.

Thanks,
Joel
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH v12 05/11] edma: config: Enable config options for EDMA

2013-06-21 Thread Joel A Fernandes 
On Fri, Jun 21, 2013 at 9:00 AM, Arnd Bergmann a...@arndb.de wrote:
 On Friday 21 June 2013, Joel A Fernandes wrote:
  diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
  index b1c66a4..7d58cd9 100644
  --- a/arch/arm/Kconfig
  +++ b/arch/arm/Kconfig
  @@ -841,6 +841,7 @@ config ARCH_DAVINCI
select HAVE_IDE
select NEED_MACH_GPIO_H
select TI_PRIV_EDMA
  + select DMADEVICES
 
  It is generally a bad idea to force select on something that can be
  enabled using menuconfig. Unless used carefully, select causes unmet
  direct dependency warnings which folks are already fighting hard to
  fix. This leads to what Russell referred in the past as select madness 
  [1]

 Are you concerned with bloat issues? I know your point of view but the idea
 was to build these options by default for these platforms even though
 in some cases
 it might not be used. I have seen folks including myself select the wrong
 option. Having the build system automatically select the correct option for 
 the
 most common cases can be very useful I feel and not require manual
 configuration.

 For defaults, you should use the defconfig, not 'select' in Kconfig.

 A lot of the 'select' statements are actually wrong because they
 do not take dependencies into account where A selects B but not C,
 and B depends on C, which leads to broken builds when C is disabled
 by a user (or randconfig).

I haven't come across this problem but- are you saying there is a
shortcoming in Kbuild/Kconfig that selects an option even if its
dependency is not met?

The problem with defconfig is also too many options I feel for a common case.
CONFIG_DMADEVICES=y
CONFIG_TI_EDMA=y

Most if not all future OMAPs from will use EDMA. Why not we can be
explicit about it and just built it in anyway. If ARCH_OMAP and
DMADEVICES are selected, then we can just build EDMA in by default.

I agree maybe the option can be dropped from Davinci but I suggest
let's keep it for OMAP. Is that ok?

Thanks,
Joel
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH v12 05/11] edma: config: Enable config options for EDMA

2013-06-21 Thread Joel A Fernandes 
On Fri, Jun 21, 2013 at 9:32 AM, Arnd Bergmann a...@arndb.de wrote:
 On Friday 21 June 2013, Joel A Fernandes wrote:
 I haven't come across this problem but- are you saying there is a
 shortcoming in Kbuild/Kconfig that selects an option even if its
 dependency is not met?

 Well, the shortcoming is that it lets you specify impossible
 contraints. You get a warning from Kconfig when building
 such a configuration, but then it continues.

 The problem with defconfig is also too many options I feel for a common case.
 CONFIG_DMADEVICES=y
 CONFIG_TI_EDMA=y

 Most if not all future OMAPs from will use EDMA. Why not we can be
 explicit about it and just built it in anyway. If ARCH_OMAP and
 DMADEVICES are selected, then we can just build EDMA in by default.

 It's just not how we do things. Kconfig is a mess because we are
 not consistent in the way this is done.

 I agree maybe the option can be dropped from Davinci but I suggest
 let's keep it for OMAP. Is that ok?

 No, I would still like you to not add it to either one. I'm spending
 a lot of my time tracking down incorrect 'select' statements and I'd
 rather spend it in a different way. I've had to a number of 'select'
 statements from OMAP in the past, please don't add any new ones
 unless there is a strict build dependency (which normally should not
 exist).

I think we are talking about different things, I agree the 'select
DMADEVICES' can be dropped but lets please keep the default y option
(not adding new select statements, just saying that if someone select
DMADEVICES in menuconfig and if they're ARCH_OMAP=1 , then default to
EDMA). This will simply allow people to have a default. Thanks.

Thanks
Joel
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH v12 05/11] edma: config: Enable config options for EDMA

2013-06-21 Thread Joel A Fernandes 
Hi Arnd,

On Fri, Jun 21, 2013 at 1:44 PM, Arnd Bergmann a...@arndb.de wrote:
 On Friday 21 June 2013, Joel A Fernandes wrote:
 I think we are talking about different things, I agree the 'select
 DMADEVICES' can be dropped but lets please keep the default y option
 (not adding new select statements, just saying that if someone select
 DMADEVICES in menuconfig and if they're ARCH_OMAP=1 , then default to
 EDMA). This will simply allow people to have a default. Thanks.

 Yes, that's ok.

Ok, thanks. I will follow up with a patch in my next submissions that builds it.

Perhaps a:
default y if 'ARCH_OMAP2PLUS'

and leave the existing as it is...
default n if  'ARCH_DAVINCI || ARCH_OMAP1 || ARCH_OMAP2'

would make most sense to me. Basically EDMA is seen on current and all
new OMAP2PLUS.


 config TI_EDMA
 tristate TI EDMA support
 default m if 'ARCH_DAVINCI || ARCH_OMAP1 || ARCH_OMAP2
 select DMA_ENGINE
 select DMA_VIRTUAL_CHANNELS



MMC depends on EDMA specially on AM33xx there's no PIO mode AFAIK. The
'm' option will require some initramfs to load the module when needing
to MMC boot, I suggest lets leave it as y.

Thanks,
Joel
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH v12 00/11] DMA Engine support for AM33XX

2013-06-21 Thread Joel A Fernandes 
Hi Sekhar,

On Fri, Jun 21, 2013 at 5:27 AM, Sekhar Nori nsek...@ti.com wrote:
 Joel,

 On 6/21/2013 2:36 AM, Joel A Fernandes wrote:
 From: Joel A Fernandes agnel.j...@gmail.com

 This series is remaining of Matt Porter's EDMA patches for AM33XX EDMA 
 support
 with changes for few pending review comments on v11 series.

 I have posted a branch with fixes for patches accepted by me in this
 series (also posted inline as replies to specific patches).

 https://git.kernel.org/cgit/linux/kernel/git/nsekhar/linux-davinci.git/log/?h=v3.11/soc-2

 Can you please give it a good test - especially on AM335x? I will send a
 pull request after you confirm. If you find bugs please send incremental
 patches so I can merge into the original patch.

This breaks on AM33XX due to dangling pointer, I sent a follow up
patch addressing it.

However, with this branch I still get..
mmc: unable to obtain RX DMA engine channel 25

Is there anything obvious you might have changed that is causing this?

Thanks,
Joel
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH v12 05/11] edma: config: Enable config options for EDMA

2013-06-21 Thread Joel A Fernandes 
  config TI_EDMA
  tristate TI EDMA support
  default m if 'ARCH_DAVINCI || ARCH_OMAP1 || ARCH_OMAP2
  select DMA_ENGINE
  select DMA_VIRTUAL_CHANNELS


 MMC depends on EDMA specially on AM33xx there's no PIO mode AFAIK. The
 'm' option will require some initramfs to load the module when needing
 to MMC boot, I suggest lets leave it as y.


 Ah, right: you still export a filter function from the edma driver
 and use it in slave drivers:

 drivers/mmc/host/davinci_mmc.c: 
 dma_request_slave_channel_compat(mask, edma_filter_fn,
 drivers/mmc/host/davinci_mmc.c: 
 dma_request_slave_channel_compat(mask, edma_filter_fn,
 drivers/spi/spi-davinci.c:  dspi-dma_rx = dma_request_channel(mask, 
 edma_filter_fn,
 drivers/spi/spi-davinci.c:  dspi-dma_tx = dma_request_channel(mask, 
 edma_filter_fn,

 As long as this is the case, you have to be careful with the dependencies
 to make sure that davinci_mmc and spi-davinci either depend on TI_EDMA, or
 edma_filter_fn gets defined to NULL when you are building for a DT-only 
 platform.

Yes sure, right now they are defined  as follows in include/linux/edma.h:

#if defined(CONFIG_TI_EDMA) || defined(CONFIG_TI_EDMA_MODULE)
bool edma_filter_fn(struct dma_chan *, void *);
#else
static inline bool edma_filter_fn(struct dma_chan *chan, void *param)
{
return false;
}
#endif

This also has the side effect of causing DMA requests to fail if
TI_EDMA is not built, causing frustration for a lot of people some of
whom don't want to deal with DMA so I think it is OK to build the
driver in by default as it is (and will be) used by a lot of
OMAP2PLUS.

Thanks,
Joel
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH v12 00/11] DMA Engine support for AM33XX

2013-06-21 Thread Joel A Fernandes 
On Fri, Jun 21, 2013 at 7:07 PM, Joel A Fernandes agnel.j...@gmail.com wrote:
 Hi Sekhar,

 On Fri, Jun 21, 2013 at 5:27 AM, Sekhar Nori nsek...@ti.com wrote:
 Joel,

 On 6/21/2013 2:36 AM, Joel A Fernandes wrote:
 From: Joel A Fernandes agnel.j...@gmail.com

 This series is remaining of Matt Porter's EDMA patches for AM33XX EDMA 
 support
 with changes for few pending review comments on v11 series.

 I have posted a branch with fixes for patches accepted by me in this
 series (also posted inline as replies to specific patches).

 https://git.kernel.org/cgit/linux/kernel/git/nsekhar/linux-davinci.git/log/?h=v3.11/soc-2

 Can you please give it a good test - especially on AM335x? I will send a
 pull request after you confirm. If you find bugs please send incremental
 patches so I can merge into the original patch.

 [Joel]
 This breaks on AM33XX due to dangling pointer, I sent a follow up
 patch addressing it.

 However, with this branch I still get..
 mmc: unable to obtain RX DMA engine channel 25

[Joel]
Fixed. I posted a v2 patch. With this, on v3.11/soc-2 branch - AM33xx
MMC is working (root fs mount).

Tested-by: Joel A Fernandes joelag...@ti.com

Thanks,
Joel
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH v11 3/8] ARM: edma: Add EDMA crossbar event mux support

2013-06-19 Thread Joel A Fernandes 
Hi Sekhar,
Thanks for the feedback.

On Tue, Jun 18, 2013 at 5:19 AM, Sekhar Nori nsek...@ti.com wrote:
 On 6/18/2013 12:08 PM, Joel A Fernandes wrote:
 From: Matt Porter mpor...@ti.com

 Changes by Joel:
 * Split EDMA xbar support out of original EDMA DT parsing patch
 to keep it easier for review.
 * Rewrite shift and offset calculation.

 Suggested-by: Sekhar Nori nsek...@ti.com
 Suggested by: Andy Shevchenko andy.shevche...@gmail.com
 Signed-off-by: Joel A Fernandes joelag...@ti.com

 Reference:
 [1] https://patchwork.kernel.org/patch/2226991/
 ---
  arch/arm/common/edma.c |   59 
 
  include/linux/platform_data/edma.h |1 +
  2 files changed, 60 insertions(+)

 diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c
 index 9823b79..1c2fb15 100644
 --- a/arch/arm/common/edma.c
 +++ b/arch/arm/common/edma.c
 @@ -1410,6 +1410,52 @@ static int edma_of_read_u32_to_s16_array(const struct 
 device_node *np,
   return 0;
  }

 +static int edma_xbar_event_map(struct device *dev,
 +struct device_node *node,
 +struct edma_soc_info *pdata, int len)
 +{
 + int ret = 0;
 + int i;
 + struct resource res;
 + void *xbar;

 void __iomem *xbar;

OK.

 + const s16 (*xbar_chans)[2];
 + u32 shift, offset, mux;
 +
 + xbar_chans = devm_kzalloc(dev,
 +   len/sizeof(s16) + 2*sizeof(s16),
 +   GFP_KERNEL);
 + if (!xbar_chans)
 + return -ENOMEM;
 +
 + ret = of_address_to_resource(node, 1, res);
 + if (ret)
 + return -EIO;
 +
 + xbar = devm_ioremap(dev, res.start, resource_size(res));
 + if (!xbar)
 + return -ENOMEM;
 +
 + ret = edma_of_read_u32_to_s16_array(node,
 + ti,edma-xbar-event-map,
 + (s16 *)xbar_chans,
 + len/sizeof(u32));
 + if (ret)
 + return -EIO;
 +
 + for (i = 0; xbar_chans[i][0] != -1; i++) {
 + shift = (xbar_chans[i][1]  0x03)  3;
 + offset = xbar_chans[i][1]  0xfffc;
 + mux = readl((void *)((u32)xbar + offset));

 Please drop unnecessary casting. Simply:

Done.


 mux = readl(xbar + offset);

 + mux = ~(0xff  shift);
 + mux |= xbar_chans[i][0]  shift;
 + writel(mux, (void *)((u32)xbar + offset));

 Fix the writel likewise.

Done.


   }

 + /* Clear the xbar mapped channels in unused list */
 + xbar_chans = info[j]-xbar_chans;
 + if (xbar_chans) {
 + for (i = 0; xbar_chans[i][1] != -1; i++) {
 + off = xbar_chans[i][1];
 + clear_bits(off, 1,
 + edma_cc[j]-edma_unused);

 Please fix the alignment here.

I noticed the alignment was off in a few other places in that driver.
Fixed those up too.

Thanks,
Joel
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 2/2] ARM: dts: add AM33XX MMC support

2013-06-15 Thread Joel A Fernandes 
Hi Felipe,

On Fri, Jun 14, 2013 at 6:43 PM, Joel A Fernandes agnel.j...@gmail.com wrote:
 Felipe,


 On Friday, June 14, 2013, Felipe Balbi wrote:

 Hi,

 On Fri, Jun 14, 2013 at 02:54:33PM -0500, Joel A Fernandes wrote:
  Hi Tony, Vaibhav,
 
   I just doublechecked MMC rootfs on bone and evmsk as it's the
   standard
   smoke test. My EVM is intermittent now so trying to coax it to power
   up
   to reverify.
  
  
   Matt,
  
   Your branch is working for me, I tested it on EVM. Not sure what is
   wrong with manual rebasing
   I did yesterday. I will try to nail-down root-cause here.
  
   And you can submit the next version fixing the comments I have given.
 
  The only comment I could find in the whole discussion was adding of
  the interrupts property. I tested I don't have any problem booting
  with Matt's patch, without having the interrupts property.

 might very well be, but we want to figure out why adding intrrupts
 causes issues. Why does it have to come through hwmod ? You might have
 uncovered a real problem which needs fixing. Besides, at the end of the


You are right, adding interrupts property to DT node causes problems
(driver probe doesn't take place). Will debug further on Monday.

Thanks,
Joel
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH v2 0/3] omap_hsmmc DT DMA Client support

2013-06-14 Thread Joel A Fernandes 
Hi,

On Wed, Mar 6, 2013 at 7:37 AM, Matt Porter mpor...@ti.com wrote:
 On Tue, Mar 05, 2013 at 09:26:01PM +, Arnd Bergmann wrote:
 On Tuesday 05 March 2013, Matt Porter wrote:
  Changes since v1:
  - rebase to 3.9-rc1, previous dependencies upstream
 
  This series adds DT DMA Engine Client support to the omap_hsmmc.
  It leverages the generic DMA OF helpers in -next and the
  dma_request_slave_channel_compat() wrapper introduced in the
  AM33XX DMA Engine series to support DMA in omap_hsmmc on platforms
  booting via DT. These platforms include omap2/3/4/5 and am33xx.
 
  These patches were split out from the v5 version of the AM33XX DMA
  series and split from the EDMA-specific omap_hsmmc changes.
 

 The description seems stale, but the patches all look good to me.

 I guess they can now get merged in any order.

 Acked-by: Arnd Bergmann a...@arndb.de

 Yes, missed updating that to indicating that those are now in
 3.9-rc1.

If there are no more changes, could this patch please be merged in for
3.10 or if not, could Matt please update the description as requested
and repost?

Thanks,
Joel
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH v2 0/3] omap_hsmmc DT DMA Client support

2013-06-14 Thread Joel A Fernandes 
Resending on Matt's new email, thanks.

On Fri, Jun 14, 2013 at 1:10 PM, Joel A Fernandes agnel.j...@gmail.com wrote:
 Hi,

 On Wed, Mar 6, 2013 at 7:37 AM, Matt Porter mpor...@ti.com wrote:
 On Tue, Mar 05, 2013 at 09:26:01PM +, Arnd Bergmann wrote:
 On Tuesday 05 March 2013, Matt Porter wrote:
  Changes since v1:
  - rebase to 3.9-rc1, previous dependencies upstream
 
  This series adds DT DMA Engine Client support to the omap_hsmmc.
  It leverages the generic DMA OF helpers in -next and the
  dma_request_slave_channel_compat() wrapper introduced in the
  AM33XX DMA Engine series to support DMA in omap_hsmmc on platforms
  booting via DT. These platforms include omap2/3/4/5 and am33xx.
 
  These patches were split out from the v5 version of the AM33XX DMA
  series and split from the EDMA-specific omap_hsmmc changes.
 

 The description seems stale, but the patches all look good to me.

 I guess they can now get merged in any order.

 Acked-by: Arnd Bergmann a...@arndb.de

 Yes, missed updating that to indicating that those are now in
 3.9-rc1.

 If there are no more changes, could this patch please be merged in for
 3.10 or if not, could Matt please update the description as requested
 and repost?

 Thanks,
 Joel
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 2/2] ARM: dts: add AM33XX MMC support

2013-06-14 Thread Joel A Fernandes 
Hi Tony, Vaibhav,

 I just doublechecked MMC rootfs on bone and evmsk as it's the standard
 smoke test. My EVM is intermittent now so trying to coax it to power up
 to reverify.


 Matt,

 Your branch is working for me, I tested it on EVM. Not sure what is wrong 
 with manual rebasing
 I did yesterday. I will try to nail-down root-cause here.

 And you can submit the next version fixing the comments I have given.

The only comment I could find in the whole discussion was adding of
the interrupts property. I tested I don't have any problem booting
with Matt's patch, without having the interrupts property.

Can this patch be pulled in for 3.11?

Tested-by: Joel A Fernandes joelag...@ti.com

Thanks,
Joel
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 2/2] ARM: dts: add AM33XX MMC support

2013-06-14 Thread Joel A Fernandes 
Felipe,

On Friday, June 14, 2013, Felipe Balbi wrote:

 Hi,

 On Fri, Jun 14, 2013 at 02:54:33PM -0500, Joel A Fernandes wrote:
  Hi Tony, Vaibhav,
 
   I just doublechecked MMC rootfs on bone and evmsk as it's the standard
   smoke test. My EVM is intermittent now so trying to coax it to power
 up
   to reverify.
  
  
   Matt,
  
   Your branch is working for me, I tested it on EVM. Not sure what is
 wrong with manual rebasing
   I did yesterday. I will try to nail-down root-cause here.
  
   And you can submit the next version fixing the comments I have given.
 
  The only comment I could find in the whole discussion was adding of
  the interrupts property. I tested I don't have any problem booting
  with Matt's patch, without having the interrupts property.

 might very well be, but we want to figure out why adding intrrupts
 causes issues. Why does it have to come through hwmod ? You might have
 uncovered a real problem which needs fixing. Besides, at the end of the


No actually I think I wasn't very descriptive in my post. There is no
problem here. Whether it comes from DT or not, it works in both cases . (I
haven't tried adding interrupt node to dts but what I gathered from
Vaibhav's post is that it works also when added to dts)

But I can revisit Vaibhav's suggestion of adding it to dts for completeness
sake. My tree currently works and picks up the interrupt info from hwmod. I
just didn't see any activity on this thread for a while so I thought of
enquiring it was ok to pull it in for the 3.11 merge window or if the
consensus was that interrupt node be added to dts even though it works
without it.

Thanks,

-Joel


 day, we want to remove the redundant hwmod data if we already have DTS
 bindings covering the same requirements.
 --
 balbi

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 4/4] ARM: dts: AM33XX: Use pinctrl constants

2013-06-02 Thread Joel A Fernandes 
Hi Florian,

This is very cool.. :-)

On Fri, May 31, 2013 at 7:42 AM, Florian Vaussard
florian.vauss...@epfl.chwrote:

 Using constants for pinctrl allows a better readability, and removes
 redundancy with comments.

 Signed-off-by: Florian Vaussard florian.vauss...@epfl.ch
 ---
  arch/arm/boot/dts/am335x-bone.dts  |   18 +-
  arch/arm/boot/dts/am335x-evm.dts   |   28 ++--
  arch/arm/boot/dts/am335x-evmsk.dts |   26 +-
  arch/arm/boot/dts/am33xx.dtsi  |1 +
  4 files changed, 37 insertions(+), 36 deletions(-)

 diff --git a/arch/arm/boot/dts/am335x-bone.dts
 b/arch/arm/boot/dts/am335x-bone.dts
 index 5bfb7dd..61d0793 100644
 --- a/arch/arm/boot/dts/am335x-bone.dts
 +++ b/arch/arm/boot/dts/am335x-bone.dts
 @@ -30,30 +30,30 @@

 user_leds_s0: user_leds_s0 {
 pinctrl-single,pins = 
 -   0x54 0x7/* gpmc_a5.gpio1_21,
 OUTPUT | MODE7 */
 -   0x58 0x17   /* gpmc_a6.gpio1_22,
 OUTPUT_PULLUP | MODE7 */
 -   0x5c 0x7/* gpmc_a7.gpio1_23,
 OUTPUT | MODE7 */
 -   0x60 0x17   /* gpmc_a8.gpio1_24,
 OUTPUT_PULLUP | MODE7 */
 +   0x54 (PIN_OUTPUT | MUX_MODE7)   /*
 gpmc_a5.gpio1_21 */
 +   0x58 (PIN_OUTPUT | PULL_UP | MUX_MODE7) /*
 gpmc_a6.gpio1_22 */
 +   0x5c (PIN_OUTPUT | MUX_MODE7)   /*
 gpmc_a7.gpio1_23 */
 +   0x60 (PIN_OUTPUT | PULL_UP | MUX_MODE7) /*
 gpmc_a8.gpio1_24 */


Does it make sense to also replace the pinmux offets with constants?
Removal of the comment would mean looking at the DTS one still has to go
back to the TRM to figure out what is the name for a particular offset.

Thanks,
Joel
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 0/6] Introducing Device Tree Overlays

2013-01-04 Thread Joel A Fernandes 
Hi Richard,

On Fri, Jan 4, 2013 at 9:35 PM, Richard Cochran
richardcoch...@gmail.com wrote:
 On Fri, Jan 04, 2013 at 09:31:04PM +0200, Pantelis Antoniou wrote:
 The following patchset introduces Device Tree overlays, a method
 of dynamically altering the kernel's live Device Tree.

 It would be nice to know the motivation for this code.

 What is the use case? What problem or issue is being addressed?

The problem being addressed is discussed in this thread:
http://permalink.gmane.org/gmane.linux.kernel/1389017

Regards,
Joel
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [RFC] Device Tree Overlays Proposal (Was Re: capebus moving omap_devices to mach-omap2)

2012-11-12 Thread Joel A Fernandes 
Hi Grant,

On Fri, Nov 9, 2012 at 3:22 PM, Grant Likely grant.lik...@secretlab.ca wrote:
 (2)
 Also this discussed a while back but at some point is going to brought
 up again-  loading of dt fragment directly from EEPROM and merging at
 run time. If we were to implement this in kernel, we would have to add
 cape specific EEPROM reading code, merge the tree before it is
 unflattened and parse.

 Unless it is required for boot to userspace I'm not considering
 merging before userspace starts. That's well after the tree is
 unflattened into the live form. If it is require to boot then I agree
 that is should be done in firmware. I see zero problem with having a
 beaglebone specific cape driver that knows to read the eeprom and
 request a specific configuration file. Heck, the kernel doesn't even
 need to parse the eeprom data. It can be read from userspace and
 userspace decides which overlay to provide. There's nothing stopping
 userspace from reading the eeprom, looking up the correct dts for the
 board, downloading the file from the Internet, compiling it with dtc
 and installing it and yes that is getting a little extreme.

Actually, I was speaking of the case where today we read EEPROM anyway
for capes before userspace, so it would convenient do the overlay this
way. Further, userspace doesn't have to do any parsing as
kernel/u-boot could blindly load the dtb overlay directly from EEPROM
regardless of which cape. So we don't have do any revision detection
and dtb search, the dtb itself being in the EEPROM.
That way no matter what the userspace is, the cape hardware is already
configured and ready to use by userspace. This might be a corner-case
just for beaglebone though, and if requesting overlay from userspace
is a more general solution for all usecases, then probably we should
go with that approach. OTOH, I guess there's no harm in doing some
overlays from kernel for some cases, and userspace for others.

 The code that does
 the tree walking can then just strcmp the node name while it walks the
 tree instead of having to find a node with a phandle number. I guess
 the reason is phandles are small to store as data values. Another
 approach can be to arrange the string block in alphabetical order
 (unless it already is), and store phandle as index of the node name
 referenced relative to the starting of the strong block. This will not
 affect nodes in dtb being moved around since they will still have the
 same index value. the problem being adding or removing nodes Changes
 the index of all other nodes in the string block as well.. Hmm.

 And that still doesn't help find all the phandle locations in the tree
 for doing fixups. It would need to be a table of
 nodes+properties+offsets that contain phandles for fixup to work, but
 I shy away from that approach because I think it will be fragile.

I guess this approach (which wouldn't work for other reasons) was
intended to serve the same purpose as Hashing, having a phandle that
doesn't easily change hence not requiring overlay fixups.

Actually I'm a bit confused if how maintaining a list of
name/node-paths for fix up will even work, because node names are not
unique as we discussed. Also node paths can change, so what will the
overlay dtb store in its fixup table (say there is one)? A table of
node,properties etc as you mentioned might not be unique as there can
be 2 nodes with same name and properties. Also if we use node path,
instead of node name, in this table, then moving nodes around wont be
possible as the fixup table will be pointing to node path references
that have now changed- how will the kernel now what's the node's new
actual path, say, if there are 2 nodes with the same name? Only thing
possible is, to do a _mandatory_ recompile of overlay dtb if a node's
path changes.

Would be you be in agreement that- When a node's path changes in base
dtb, Overlays have to be recompiled and a new fixup table has to be
created, or a new hash has to be generated out of the new node path
regardless. So moving nodes around in base dtb can't be dynamically
accommodated without recompiling overlay.

Considering the above is ok, the only 2 approaches I can see for
phandle resolutions are:
(1) hashing of the full node path and avoiding runtime fixups.
(2) Using today's linear phandle increment approach, maintaining a
table in the overlay with the key as 'node path' and value as 'phandle
fixup offset', and then doing runtime fixups.

I believe (2) is considered fragile and it does the same thing as (1)
except that (2) can even handle collisions but at the cost of storing
the full node path in the overlay's fixup table. So (1) looks like our
only possible and meaningful approach. Do you agree with this
analysis? Or, maybe I am missing something.

Regards,
Joel
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [RFC] Device Tree Overlays Proposal (Was Re: capebus moving omap_devices to mach-omap2)

2012-11-09 Thread Joel A Fernandes 
On Fri, Nov 9, 2012 at 8:29 AM, David Gibson
da...@gibson.dropbear.id.au wrote:
 On Fri, Nov 09, 2012 at 12:32:09AM -0500, Joel A Fernandes wrote:
 Hi Pantelis,

 I hope I'm not too late to reply as I'm traveling.

 On Nov 6, 2012, at 5:30 AM, Pantelis Antoniou
 pa...@antoniou-consulting.com wrote:

  Joanne has purchased one of Jane's capes and packaged it into a rugged
  case for data logging. As far as Joanne is concerned, the BeagleBone and
  cape together are a single unit and she'd prefer a single monolithic FDT
  instead of using an FDT overlay.
  Option A: Using dtc, she uses the BeagleBone and cape .dts source files
 to generate a single .dtb for the entire system which is
 loaded by U-Boot. -or-
 
  Unlikely.
  Option B: Joanne uses a tool to merge the BeagleBone and cape .dtb files
 (instead of .dts files), -or-
  Possible but low probability.
 
  Option C: U-Boot loads both the base and overlay FDT files, merges them,
 and passes the resolved tree to the kernel.
 
 
  Could be made to work. Only really required if Joanne wants the
  cape interface to work for u-boot too. For example if the cape has some
  kind of network interface that u-boot will use to boot from.
 

 I love Grant's hashing idea a lot keeping the phandle problem for
 compile time and not requiring fixups.

 Well, using a hash only moves the problem of fixed phandles to a
 problem of fixed node paths.  The details of node paths are, if
 anything, more mutable than phandles.

So what you're saying is there's no way to make a phandle a signature
of a (property of a node) since no one property is unique. If I
follow, even node path can't be used because hash value changes if
node is moved around in the tree. This shoots down the hashing idea
then, which I guess is looked down upon anyway due to dynamic changes
to the base DT as discussed in the usecases.

 [snip]
 Alternatively to hashing, reading David Gibson's paper I followed,
 phandle is supposed to 'uniquely' identity node. I wonder why the node
 name itself is not sufficient to uniquely identify.

 Node names are not unique, not even close.  If you have two similar
 NICs in slot 0 of two different PCI domains, they'll almost certainly
 both be called 'ethernet@0,0'.  Similar examples abound on other
 buses.  Node paths are unique, but they are long.

 The other big reason for phandles in OF history is that they would be
 more stable than paths.  The device tree could be manipulated during
 OF runtime, but phandles would generally be internal pointers in OF
 and so remain a consistent handle even if the node moved in the tree.
 That's not really relevant for flat trees, but we need to work with
 the same structures.

Thanks.

 The code that does
 the tree walking can then just strcmp the node name while it walks the
 tree instead of having to find a node with a phandle number. I guess
 the reason is phandles are small to store as data values. Another
 approach can be to arrange the string block in alphabetical order
 (unless it already is),

 They're not, and doing so would be a painful change to maintain
 compatibility across.  And in any case only property names use the
 strings block, not node names.

Understood, thanks.

Regards,
Joel
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [RFC] Device Tree Overlays Proposal (Was Re: capebus moving omap_devices to mach-omap2)

2012-11-09 Thread Joel A Fernandes 
Hi Pantelis,

On Fri, Nov 9, 2012 at 2:13 AM, Pantelis Antoniou
pa...@antoniou-consulting.com wrote:

 Option C: U-Boot loads both the base and overlay FDT files, merges them,
   and passes the resolved tree to the kernel.


 Could be made to work. Only really required if Joanne wants the
 cape interface to work for u-boot too. For example if the cape has some
 kind of network interface that u-boot will use to boot from.


 I love Grant's hashing idea a lot keeping the phandle problem for
 compile time and not requiring fixups.

 IMO it is still a cleaner approach if u-boot does the tree merging for
 all cases, and not the kernel.

 That way from a development standpoint, very little or nothing will
 have to be changed in kernel (except for scripts/dtc) considering we
 are moving forward with hashing.

 Also this discussed a while back but at some point is going to brought
 up again-  loading of dt fragment directly from EEPROM and merging at
 run time. If we were to implement this in kernel, we would have to add
 cape specific EEPROM reading code, merge the tree before it is
 unflattened and parse. I think doing tree merging in kernel is messy
 and we should do it in uboot. Ideally reading the fragment from the
 EEPROM for all capes and merging without worrying about version
 detection, Doing the merge and passing the merged blob to the kernel
 which (kernel) works the same way it does today.

 Not going to work, for a lot of cases. Doing it in the kernel seems to be
 the cleaner option. There are valid use cases for doing in u-boot too.

True, if dynamic runtime stuff from userspace is what we're talking
about, then yeah I see the important need for kernel to do the merge.

 Alternatively to hashing, reading david gibsons paper I followed,
 phandle is supposed to 'uniquely' identity node. I wonder why the node
 name itself is not sufficient to unquiely identify. The code that does
 the tree walking can then just strcmp the node name while it walks the
 tree instead of having to find a node with a phandle number. I guess
 the reason is phandles are small to store as data values. Another
 approach can be to arrange the string block in alphabetical order
 (unless it already is), and store phandle as index of the node name
 referenced relative to the starting of the strong block. This will not
 affect nodes in dtb being moved around since they will still have the
 same index value. the problem being adding or removing nodes Changes
 the offsets of all other nodes in the string block as well.. Hmm.


 This is pretty radical change to the DT format, no?

Yes, true and the only way hypothetically to replace the phandle
tree-walking mechanism is to store node paths instead of phandle which
David pointed is too long to store, so I guess this wont work after
all. Anyway this was an interesting exercise, thanks.

Regards,
Joel
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [RFC] Device Tree Overlays Proposal (Was Re: capebus moving omap_devices to mach-omap2)

2012-11-08 Thread Joel A Fernandes 
Hi Pantelis,

I hope I'm not too late to reply as I'm traveling.

On Nov 6, 2012, at 5:30 AM, Pantelis Antoniou
pa...@antoniou-consulting.com wrote:



 Joanne has purchased one of Jane's capes and packaged it into a rugged
 case for data logging. As far as Joanne is concerned, the BeagleBone and
 cape together are a single unit and she'd prefer a single monolithic FDT
 instead of using an FDT overlay.
 Option A: Using dtc, she uses the BeagleBone and cape .dts source files
to generate a single .dtb for the entire system which is
loaded by U-Boot. -or-

 Unlikely.
 Option B: Joanne uses a tool to merge the BeagleBone and cape .dtb files
(instead of .dts files), -or-
 Possible but low probability.

 Option C: U-Boot loads both the base and overlay FDT files, merges them,
and passes the resolved tree to the kernel.


 Could be made to work. Only really required if Joanne wants the
 cape interface to work for u-boot too. For example if the cape has some
 kind of network interface that u-boot will use to boot from.


I love Grant's hashing idea a lot keeping the phandle problem for
compile time and not requiring fixups.

IMO it is still a cleaner approach if u-boot does the simple tree merging for
all cases, and not the kernel reasons mentioned below.

(1)
From a development standpoint, very little or nothing will
have to be changed in kernel (except for scripts/dtc) considering we
are moving forward with hashing.

(2)
Also this discussed a while back but at some point is going to brought
up again-  loading of dt fragment directly from EEPROM and merging at
run time. If we were to implement this in kernel, we would have to add
cape specific EEPROM reading code, merge the tree before it is
unflattened and parse. I think doing tree merging in kernel is messy
and we should do it in uboot considering we might have to read EEPROM for
this use case. Ideally reading the fragment from the EEPROM for all capes
and merging without worrying about version detection, Doing the merge and
passing the merged blob to the kernel which (kernel) works the same way
it does today.

 It may be sufficient to solve it by making the phandle values less
 volatile. Right now dtc generates phandles linearly. Generated phandles
 could be overridden with explicit phandle properties, but it isn't a
 fantastic solution. Perhaps generating the phandle from a hash of the
 node name would be sufficient.


 I doubt the hash method will work reliably. We only have 32 bits to work with,
 nothing like the SHA hashes of git.


I was wondering I have worked with kernel's crypto code in the past to
generate 32 bit md5sums of 1000s of dataitems, from what I've seen,
collisions are rare and since we are talking about just a few nodes
that are being referenced in the base dt. I think the probability is
even less (ofcourse such an analysis strongly depends on dataset).
this method also takes away a lot of complexity with having it to do
runtime fixups and will help us get off the ground quickly.

We can also put in a collision handling mechanism if needed.
I think it is worthy doing a sample hash of all nodes in all dts we
have in a script and see for once if we have collisions and what it
looks like.

Alternatively to hashing, reading David Gibson's paper I followed,
phandle is supposed to 'uniquely' identity node. I wonder why the node
name itself is not sufficient to uniquely identify. The code that does
the tree walking can then just strcmp the node name while it walks the
tree instead of having to find a node with a phandle number. I guess
the reason is phandles are small to store as data values. Another
approach can be to arrange the string block in alphabetical order
(unless it already is), and store phandle as index of the node name
referenced relative to the starting of the strong block. This will not
affect nodes in dtb being moved around since they will still have the
same index value. the problem being adding or removing nodes Changes
the index of all other nodes in the string block as well.. Hmm.

Regards,
Joel
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

2012-11-05 Thread Joel A Fernandes 
Hi Grant,

On Mon, Nov 5, 2012 at 2:14 PM, Grant Likely grant.lik...@secretlab.ca wrote:
 On Mon, Nov 5, 2012 at 7:54 PM, Pantelis Antoniou
 pa...@antoniou-consulting.com wrote:
 This handles many of the use cases, but it assumes that an overlay is
 board specific. If it ever is required to support multiple base boards
 with a single overlay file then there is a problem. The .dtb overlays
 generated in this manor cannot handle different phandles or nodes that
 are in a different place. On the other hand, the overlay source files
 should have no problem being compiled for multiple targets, so maybe
 it isn't an issue. Plus if dtc is installed on the target, then the
 live tree from /proc can be used as the reference when compiling the
 overlay.

 My worry is that this format is dependent on linking against the board
 DTS file. One of the ideas thrown around here was that it might make
 sense to store the DTB fragment in the EEPROM of the device.

 Right, that wouldn't work well if the base DT changed, or if a
 BeagleBone2 is released that has the same header configuration, but
 different backing devices. It would be nice to have a solution for
 that.

 In that case you have a OS independent hardware description, which can
 be even used even by the bootloader to access devices it knows not about
 at compile time.

 Other than that, I have no other objections.

 I'm open to suggestions if anyone has any. I have not objections to a
 fixup approach, but I'm not comfortable with anything that is fragile
 to modifications to the fragment.

I am fairly new to the DT world so please bear with me, but how about
a method that resolves symbols the same way that the linux dynamic
linker does when shared libraries are loaded?

A separate table (similar to .PLT/GOT sections  in the ELF object
format) could be created when the fragment is loaded, and the phandle
references could be fixed to point to the table offsets during compile
time. This table would be a second level indirection and the kernel
would populate this table with the in-kernel values of the phandles
that the dt fragment refers to.

This might involve changes to the DT core, but as such, this method
wouldn't suffer from the fragility problem of either base or fragment
DT trees being modified.

The table itself could be added to the tree by the compiler, and the
phandles could point to it (fixed). such phandles could be marked for
special handling to facilitate the 1-level indirection.

What do you think about this?

Regards,
Joel
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 0/3] capebus moving omap_devices to mach-omap2

2012-11-05 Thread Joel A Fernandes 
Hi Grant,

On Mon, Nov 5, 2012 at 5:58 PM, Grant Likely grant.lik...@secretlab.ca wrote:


 Joel A Fernandes agnel.j...@gmail.com wrote:

Hi Grant,

On Mon, Nov 5, 2012 at 2:14 PM, Grant Likely
grant.lik...@secretlab.ca wrote:
 I'm open to suggestions if anyone has any. I have not objections to a
 fixup approach, but I'm not comfortable with anything that is fragile
 to modifications to the fragment.

I am fairly new to the DT world so please bear with me, but how about
a method that resolves symbols the same way that the linux dynamic
linker does when shared libraries are loaded?

A separate table (similar to .PLT/GOT sections  in the ELF object
format) could be created when the fragment is loaded, and the phandle
references could be fixed to point to the table offsets during compile
time. This table would be a second level indirection and the kernel
would populate this table with the in-kernel values of the phandles
that the dt fragment refers to.

 That's an interesting idea that is worth exploring. That would make
 it possible to avoid a fixup stage, but it also means that any parsing
 code must know how to handle the got-like table. It won't be
 backwards compatible with existing tools. It also wouldn't easily
 support the case of firmware applying the overlay and passing the
 resulting tree to the kernel. Hmmm Not being backwards compatible
 at the data level is a big problem. I really want a method that can
 resolve back to the current data format or is a compatible extension
 of it.


Is it meaningful or feasible to make the table a part of the tree
structure itself? the table would initially be empty. If I'm right,
this is how the GOT tables in ELF objects that refer to unresolved
symbols in shared libraries are implemented as well, they are a part
of the object and are loaded into memory and fixed up. If the table is
a part of the DT data, the tools would then be able to parse such a
tree. We could enforce that the got-like tree would strictly follow a
predefined format.

Something along these lines would also avoid the need for a tree fix
up. Do you think this reduces the difficulty of backward compatibility
with the parsing tools and firmware loading?


This might involve changes to the DT core, but as such, this method
wouldn't suffer from the fragility problem of either base or fragment
DT trees being modified.

The table itself could be added to the tree by the compiler, and the
phandles could point to it (fixed). such phandles could be marked for
special handling to facilitate the 1-level indirection.

 That's part of the problem. Property values are essentially
 anaonymous data. There is no mechanism currently for marking data
 such as indicate which data values are phandles. If there were then
 it would be a simple matter to find all the phandles and fix them up.

 We could possibly add data type suppplementary properties to the tree
 to solve that problem. They would have to be optional for the base
 tree to retain backwards compatibility, but could be required on
 overlays.


Sure, so if we add data type supplementary properties to the tree to
indicate the data type as indirect phandle, then kernel could refer
to the index in the got-like table to fetch the actual phandle address
(1-level of indirection), instead of directly using the address in the
data field.


Thanks and Regards,

Joel
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss