[PATCH] of: make sure of_alias is initialized before accessing it

[Laurentiu Tudor]

Simply swap of_alias and of_chosen initialization so
that of_alias ends up read first. This must be done
because it is accessed couple of lines below when
trying to initialize the of_stdout using the alias
based legacy method.

[Fixes a752ee5 - tty: Update hypervisor tty drivers to  
use core stdout parsing code]

Signed-off-by: Laurentiu Tudor laurentiu.tu...@freescale.com
Cc: Grant Likely grant.lik...@linaro.org
---
 drivers/of/base.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index d8574ad..52f8506 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1847,6 +1847,10 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64 
align))
 {
struct property *pp;
 
+   of_aliases = of_find_node_by_path(/aliases);
+   if (!of_aliases)
+   return;
+
of_chosen = of_find_node_by_path(/chosen);
if (of_chosen == NULL)
of_chosen = of_find_node_by_path(/chosen@0);
@@ -1862,10 +1866,6 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64 
align))
of_stdout = of_find_node_by_path(name);
}
 
-   of_aliases = of_find_node_by_path(/aliases);
-   if (!of_aliases)
-   return;
-
for_each_property_of_node(of_aliases, pp) {
const char *start = pp-name;
const char *end = start + strlen(start);
-- 
1.8.3.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] of: make sure of_alias is initialized before accessing it

[Laurentiu Tudor]

On 09/08/2014 04:29 PM, Grant Likely wrote:
 On Wed, 27 Aug 2014 17:09:39 +0300, Laurentiu Tudor b10...@freescale.com 
 wrote:
 Simply swap of_alias and of_chosen initialization so
 that of_alias ends up read first. This must be done
 because it is accessed couple of lines below when
 trying to initialize the of_stdout using the alias
 based legacy method.

 [Fixes a752ee5 - tty: Update hypervisor tty drivers to   

 use core stdout parsing code]

 Signed-off-by: Laurentiu Tudor laurentiu.tu...@freescale.com
 Cc: Grant Likely grant.lik...@linaro.org
 ---
  drivers/of/base.c | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)

 diff --git a/drivers/of/base.c b/drivers/of/base.c
 index d8574ad..52f8506 100644
 --- a/drivers/of/base.c
 +++ b/drivers/of/base.c
 @@ -1847,6 +1847,10 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64 
 align))
  {
  struct property *pp;
  
 +of_aliases = of_find_node_by_path(/aliases);
 +if (!of_aliases)
 +return;
 +
  of_chosen = of_find_node_by_path(/chosen);
  if (of_chosen == NULL)
  of_chosen = of_find_node_by_path(/chosen@0);
 @@ -1862,10 +1866,6 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64 
 align))
  of_stdout = of_find_node_by_path(name);
  }
  
 -of_aliases = of_find_node_by_path(/aliases);
 -if (!of_aliases)
 -return;
 -
 
 Close, but not quite. The 'if (!of_aliases)' test should not be moved.
 Only the search for of_find_node_by_path().

Eek, completely missed this. Sorry.

 I've fixed it up and applied.
 
Thanks!

---
Best Regards, Laurentiu
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 03/15] powerpc/msi: Improve IRQ bitmap allocator

[Laurentiu Tudor]

On 09/19/2014 11:16 PM, Scott Wood wrote:
 On Thu, 2014-09-18 at 18:26 +1000, Michael Neuling wrote:
 From: Ian Munsie imun...@au1.ibm.com

 Currently msi_bitmap_alloc_hwirqs() will round up any IRQ allocation requests
 to the nearest power of 2.  eg. ask for 5 IRQs and you'll get 8.  This 
 wastes a
 lot of IRQs which can be a scarce resource.

 For cxl we can require multiple IRQs for every contexts that is attached to 
 the
 accelerator.  For AFU directed accelerators, there may be 1000s of contexts
 attached, hence we can easily run out of IRQs, especially if we are 
 needlessly
 wasting them.

 This changes the msi_bitmap_alloc_hwirqs() to allocate only the required 
 number
 of IRQs, hence avoiding this wastage.

 Signed-off-by: Ian Munsie imun...@au1.ibm.com
 Signed-off-by: Michael Neuling mi...@neuling.org
 ---
  arch/powerpc/sysdev/msi_bitmap.c | 18 +-
  1 file changed, 13 insertions(+), 5 deletions(-)
 
 This conflicts with (and partially duplicates)
 http://patchwork.ozlabs.org/patch/381892/
 which I have in my tree.  How should we handle it?
 
 Laurentiu, from looking at the overlap between patches I see a problem
 with your existing patch, regarding the out-of-irqs path and
 msi_bitmap_free_hwirqs(), so one way or another that needs to get fixed
 soon.
 

Agree. My patch lacks error checking so Michael's patch is better.

---
Best Regards, Laurentiu

 
 diff --git a/arch/powerpc/sysdev/msi_bitmap.c 
 b/arch/powerpc/sysdev/msi_bitmap.c
 index 2ff6302..e001559 100644
 --- a/arch/powerpc/sysdev/msi_bitmap.c
 +++ b/arch/powerpc/sysdev/msi_bitmap.c
 @@ -24,28 +24,36 @@ int msi_bitmap_alloc_hwirqs(struct msi_bitmap *bmp, int 
 num)
   * This is fast, but stricter than we need. We might want to add
   * a fallback routine which does a linear search with no alignment.
   */
 -offset = bitmap_find_free_region(bmp-bitmap, bmp-irq_count, order);
 +offset = bitmap_find_next_zero_area(bmp-bitmap, bmp-irq_count, 0,
 +num, (1  order) - 1);
 +if (offset  bmp-irq_count)
 +goto err;
 +bitmap_set(bmp-bitmap, offset, num);
  spin_unlock_irqrestore(bmp-lock, flags);
  
  pr_debug(msi_bitmap: allocated 0x%x (2^%d) at offset 0x%x\n,
   num, order, offset);
  
  return offset;
 +err:
 +spin_unlock_irqrestore(bmp-lock, flags);
 +return -ENOMEM;
  }
 +EXPORT_SYMBOL(msi_bitmap_alloc_hwirqs);
  
  void msi_bitmap_free_hwirqs(struct msi_bitmap *bmp, unsigned int offset,
  unsigned int num)
  {
  unsigned long flags;
 -int order = get_count_order(num);
  
 -pr_debug(msi_bitmap: freeing 0x%x (2^%d) at offset 0x%x\n,
 - num, order, offset);
 +pr_debug(msi_bitmap: freeing 0x%x at offset 0x%x\n,
 + num, offset);
  
  spin_lock_irqsave(bmp-lock, flags);
 -bitmap_release_region(bmp-bitmap, offset, order);
 +bitmap_clear(bmp-bitmap, offset, num);
  spin_unlock_irqrestore(bmp-lock, flags);
  }
 +EXPORT_SYMBOL(msi_bitmap_free_hwirqs);
  
  void msi_bitmap_reserve_hwirq(struct msi_bitmap *bmp, unsigned int hwirq)
  {
 
 

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 03/15] powerpc/msi: Improve IRQ bitmap allocator

[Laurentiu Tudor]

On 09/19/2014 11:19 PM, Scott Wood wrote:
 On Fri, 2014-09-19 at 15:16 -0500, Scott Wood wrote:
 On Thu, 2014-09-18 at 18:26 +1000, Michael Neuling wrote:
 From: Ian Munsie imun...@au1.ibm.com

 Currently msi_bitmap_alloc_hwirqs() will round up any IRQ allocation 
 requests
 to the nearest power of 2.  eg. ask for 5 IRQs and you'll get 8.  This 
 wastes a
 lot of IRQs which can be a scarce resource.

 For cxl we can require multiple IRQs for every contexts that is attached to 
 the
 accelerator.  For AFU directed accelerators, there may be 1000s of contexts
 attached, hence we can easily run out of IRQs, especially if we are 
 needlessly
 wasting them.

 This changes the msi_bitmap_alloc_hwirqs() to allocate only the required 
 number
 of IRQs, hence avoiding this wastage.

 Signed-off-by: Ian Munsie imun...@au1.ibm.com
 Signed-off-by: Michael Neuling mi...@neuling.org
 ---
  arch/powerpc/sysdev/msi_bitmap.c | 18 +-
  1 file changed, 13 insertions(+), 5 deletions(-)

 This conflicts with (and partially duplicates)
 http://patchwork.ozlabs.org/patch/381892/
 which I have in my tree.  How should we handle it?

 Laurentiu, from looking at the overlap between patches I see a problem
 with your existing patch, regarding the out-of-irqs path and
 msi_bitmap_free_hwirqs(), so one way or another that needs to get fixed
 soon.
 
 Given the problems with Laurentiu's patch, perhaps it'd be best for me
 to just revert that patch in my tree, and respin it after this patchset
 has been merged.

Let me know if you want me to rebase my stuff on top of Michael's patch.

---
Best Regards, Laurentiu

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 03/15] powerpc/msi: Improve IRQ bitmap allocator

[Laurentiu Tudor]

Hi Michael,

Minor comment inline.

On 09/18/2014 11:26 AM, Michael Neuling wrote:
 From: Ian Munsie imun...@au1.ibm.com
 
 Currently msi_bitmap_alloc_hwirqs() will round up any IRQ allocation requests
 to the nearest power of 2.  eg. ask for 5 IRQs and you'll get 8.  This wastes 
 a
 lot of IRQs which can be a scarce resource.
 
 For cxl we can require multiple IRQs for every contexts that is attached to 
 the
 accelerator.  For AFU directed accelerators, there may be 1000s of contexts
 attached, hence we can easily run out of IRQs, especially if we are needlessly
 wasting them.
 
 This changes the msi_bitmap_alloc_hwirqs() to allocate only the required 
 number
 of IRQs, hence avoiding this wastage.
 
 Signed-off-by: Ian Munsie imun...@au1.ibm.com
 Signed-off-by: Michael Neuling mi...@neuling.org
 ---
  arch/powerpc/sysdev/msi_bitmap.c | 18 +-
  1 file changed, 13 insertions(+), 5 deletions(-)
 
 diff --git a/arch/powerpc/sysdev/msi_bitmap.c 
 b/arch/powerpc/sysdev/msi_bitmap.c
 index 2ff6302..e001559 100644
 --- a/arch/powerpc/sysdev/msi_bitmap.c
 +++ b/arch/powerpc/sysdev/msi_bitmap.c
 @@ -24,28 +24,36 @@ int msi_bitmap_alloc_hwirqs(struct msi_bitmap *bmp, int 
 num)
* This is fast, but stricter than we need. We might want to add
* a fallback routine which does a linear search with no alignment.
*/

Is this comment still relevant (especially the part mentioning fast)?

---
Best Regards, Laurentiu
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCHv4] clk: ppc-corenet: rename to ppc-qoriq and add CLK_OF_DECLARE support

[Laurentiu Tudor]

Hi Jingchang,

On 09/23/2014 09:46 AM, Jingchang Lu wrote:
 The IP is shared by PPC and ARM, this renames it to qoriq for better
 represention, and this also adds the CLK_OF_DECLARE support for being
 initialized by of_clk_init() on ARM.
 

I think you need to also update drivers/cpufreq/Kconfig.powerpc
to use the renamed config option.

---
Best Regards, Laurentiu
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 5/8] arm64: kernel: replace early 64-bit literal loads with move-immediates

[Laurentiu Tudor]

On 04/18/2016 06:09 PM, Ard Biesheuvel wrote:
> When building a relocatable kernel, we currently rely on the fact that
> early 64-bit literal loads need to be deferred to after the relocation
> has been performed only if they involve symbol references, and not if
> they involve assemble time constants. While this is not an unreasonable
> assumption to make, it is better to switch to movk/movz sequences, since
> these are guaranteed to be resolved at link time, simply because there are
> no dynamic relocation types to describe them.
> 
> Signed-off-by: Ard Biesheuvel 
> ---
>  arch/arm64/kernel/head.S | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index 0d487d90d221..dae9cabaadf5 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -337,7 +337,7 @@ __create_page_tables:
>   cmp x0, x6
>   b.lo1b
>  
> - ldr x7, =SWAPPER_MM_MMUFLAGS
> + mov x7, SWAPPER_MM_MMUFLAGS

mov_q here too?

---
Best Regards, Laurentiu

>  
>   /*
>* Create the identity mapping.
> @@ -393,7 +393,7 @@ __create_page_tables:
>* Map the kernel image (starting with PHYS_OFFSET).
>*/
>   mov x0, x26 // swapper_pg_dir
> - ldr x5, =KIMAGE_VADDR
> + mov_q   x5, KIMAGE_VADDR
>   add x5, x5, x23 // add KASLR displacement
>   create_pgd_entry x0, x5, x3, x6
>   ldr w6, =kernel_img_size
> @@ -631,7 +631,7 @@ ENTRY(secondary_holding_pen)
>   bl  el2_setup   // Drop to EL1, 
> w20=cpu_boot_mode
>   bl  set_cpu_boot_mode_flag
>   mrs x0, mpidr_el1
> - ldr x1, =MPIDR_HWID_BITMASK
> + mov_q   x1, MPIDR_HWID_BITMASK
>   and x0, x0, x1
>   adr_l   x3, secondary_holding_pen_release
>  pen: ldr x4, [x3]
> @@ -773,7 +773,7 @@ __primary_switch:
>   ldr w9, =__rela_offset  // offset to reloc table
>   ldr w10, =__rela_size   // size of reloc table
>  
> - ldr x11, =KIMAGE_VADDR  // default virtual offset
> + mov_q   x11, KIMAGE_VADDR   // default virtual offset
>   add x11, x11, x23   // actual virtual offset
>   add x8, x8, x11 // __va(.dynsym)
>   add x9, x9, x11 // __va(.rela)
> 

Re: [PATCH 4/9] staging: fsl-mc: don't use devres api for refcounted objects

[Laurentiu Tudor]

On 02/03/2017 02:02 AM, Stuart Yoder wrote:
>
>
>> -Original Message-
>> From: laurentiu.tu...@nxp.com [mailto:laurentiu.tu...@nxp.com]
>> Sent: Wednesday, February 01, 2017 5:43 AM
>> To: gre...@linuxfoundation.org
>> Cc: de...@driverdev.osuosl.org; linux-kernel@vger.kernel.org; ag...@suse.de; 
>> a...@arndb.de; Ioana
>> Ciornei <ioana.cior...@nxp.com>; Ruxandra Ioana Radulescu 
>> <ruxandra.radule...@nxp.com>; Bharat Bhushan
>> <bharat.bhus...@nxp.com>; Stuart Yoder <stuart.yo...@nxp.com>; Catalin 
>> Horghidan
>> <catalin.horghi...@nxp.com>; Leo Li <leoyang...@nxp.com>; Roy Pledge 
>> <roy.ple...@nxp.com>; Laurentiu
>> Tudor <laurentiu.tu...@nxp.com>
>> Subject: [PATCH 4/9] staging: fsl-mc: don't use devres api for refcounted 
>> objects
>>
>> From: Laurentiu Tudor <laurentiu.tu...@nxp.com>
>>
>> Mixing two memory management systems, in this case
>> managed device resource api and refcounted objects
>> is a bad idea. Lifetime of an object is controlled
>> by its refcount so allocating it with other apis
>> that have their own lifetime control is not ok.
>> Drop devm_*() apis in favor of plain allocations.
>>
>> While at it, let's drop the slab cache for objects
>> until we actually have proof that it improves
>> performance. This allows for some code cleanup.
>
> Those 2 changes (dropping devm_* apis and slab cache
> changes) are 2 orthogonal things, right?  It would
> be better to split those into separate patches.  Mixing
> them makes it harder to review.
>
>> Signed-off-by: Laurentiu Tudor <laurentiu.tu...@nxp.com>
>> ---
>>   drivers/staging/fsl-mc/bus/fsl-mc-bus.c | 43 
>> +
>>   1 file changed, 6 insertions(+), 37 deletions(-)
>>
>> diff --git a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c 
>> b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
>> index 6601bde..c493427 100644
>> --- a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
>> +++ b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
>> @@ -27,8 +27,6 @@
>>   #include "fsl-mc-private.h"
>>   #include "dprc-cmd.h"
>>
>> -static struct kmem_cache *mc_dev_cache;
>> -
>>   /**
>>* Default DMA mask for devices on a fsl-mc bus
>>*/
>> @@ -422,17 +420,12 @@ bool fsl_mc_is_root_dprc(struct device *dev)
>>   static void fsl_mc_device_release(struct device *dev)
>>   {
>>  struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
>> -struct fsl_mc_bus *mc_bus = NULL;
>>
>>  kfree(mc_dev->regions);
>> -
>> -if (strcmp(mc_dev->obj_desc.type, "dprc") == 0)
>> -mc_bus = to_fsl_mc_bus(mc_dev);
>> -
>> -if (mc_bus)
>> -devm_kfree(mc_dev->dev.parent, mc_bus);
>> +if (!strcmp(mc_dev->obj_desc.type, "dprc"))
>> +kfree(to_fsl_mc_bus(mc_dev));
>>  else
>> -kmem_cache_free(mc_dev_cache, mc_dev);
>> +kfree(mc_dev);
>>   }
>>
>>   /**
>> @@ -457,7 +450,7 @@ int fsl_mc_device_add(struct dprc_obj_desc *obj_desc,
>>  /*
>>   * Allocate an MC bus device object:
>>   */
>> -mc_bus = devm_kzalloc(parent_dev, sizeof(*mc_bus), GFP_KERNEL);
>> +mc_bus = kzalloc(sizeof(*mc_bus), GFP_KERNEL);
>>  if (!mc_bus)
>>  return -ENOMEM;
>>
>> @@ -466,7 +459,7 @@ int fsl_mc_device_add(struct dprc_obj_desc *obj_desc,
>>  /*
>>   * Allocate a regular fsl_mc_device object:
>>   */
>> -mc_dev = kmem_cache_zalloc(mc_dev_cache, GFP_KERNEL);
>> +mc_dev = kzalloc(sizeof(*mc_dev), GFP_KERNEL);
>>  if (!mc_dev)
>>  return -ENOMEM;
>>  }
>> @@ -561,10 +554,7 @@ int fsl_mc_device_add(struct dprc_obj_desc *obj_desc,
>>
>>   error_cleanup_dev:
>>  kfree(mc_dev->regions);
>> -if (mc_bus)
>> -devm_kfree(parent_dev, mc_bus);
>> -else
>> -kmem_cache_free(mc_dev_cache, mc_dev);
>> +kfree(mc_bus ? (void *)mc_bus : (void *)mc_dev);
>>
>>  return error;
>>   }
>> @@ -578,23 +568,11 @@ int fsl_mc_device_add(struct dprc_obj_desc *obj_desc,
>>*/
>>   void fsl_mc_device_remove(struct fsl_mc_device *mc_dev)
>>   {
>> -struct fsl_mc_bus *mc_bus = NULL;
>> -
>> -kfree(mc_dev->regions);
>> -
>>  /*
>>   * The device-specific remove callback will get invoked by device_del()
>>   */
>>  device_del(_dev->dev);
>>  put_device(_dev->dev);
>> -
>> -if (strcmp(mc_dev->obj_desc.type, "dprc") == 0)
>> -mc_bus = to_fsl_mc_bus(mc_dev);
>> -
>> -if (mc_bus)
>> -devm_kfree(mc_dev->dev.parent, mc_bus);
>> -else
>> -kmem_cache_free(mc_dev_cache, mc_dev);
>>   }
>
> The above changes in fsl_mc_device_remove(), I think
> actually belong in patch #3 of this series.
>

Agree. I think I end up doing a double free here.

---
Thanks & Best Regards, Laurentiu

Re: [PATCH 2/9] staging: fsl-mc: fix device ref counting

[Laurentiu Tudor]

Hi Greg,

Thanks for having a look. Comment below.

On 02/03/2017 11:56 AM, Greg KH wrote:
> On Wed, Feb 01, 2017 at 05:43:22AM -0600, laurentiu.tu...@nxp.com wrote:
>> From: Laurentiu Tudor <laurentiu.tu...@nxp.com>
>>
>> Drop unneeded get_device() call at device creation
>> and, as per documentation, drop reference count
>> after using device_find_child() return.
>>
>> Signed-off-by: Laurentiu Tudor <laurentiu.tu...@nxp.com>
>> ---
>>   drivers/staging/fsl-mc/bus/dprc-driver.c | 1 +
>>   drivers/staging/fsl-mc/bus/fsl-mc-bus.c  | 1 -
>>   2 files changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c 
>> b/drivers/staging/fsl-mc/bus/dprc-driver.c
>> index 4e416d8..e4b0341 100644
>> --- a/drivers/staging/fsl-mc/bus/dprc-driver.c
>> +++ b/drivers/staging/fsl-mc/bus/dprc-driver.c
>> @@ -188,6 +188,7 @@ static void dprc_add_new_devices(struct fsl_mc_device 
>> *mc_bus_dev,
>>  child_dev = fsl_mc_device_lookup(obj_desc, mc_bus_dev);
>>  if (child_dev) {
>>  check_plugged_state_change(child_dev, obj_desc);
>> +put_device(_dev->dev);
>>  continue;
>>  }
>>
>> diff --git a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c 
>> b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
>> index cc20dc4..7c6a43b 100644
>> --- a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
>> +++ b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
>> @@ -537,7 +537,6 @@ int fsl_mc_device_add(struct dprc_obj_desc *obj_desc,
>>  goto error_cleanup_dev;
>>  }
>>
>> -(void)get_device(_dev->dev);
>
> This implies that your device reference counting is totally wrong and
> messed up.  Does this fix anything?  Break anything?  It should do
> something different now...

It fixes the refcounting in the sense that I'm now seeing the error
that i think you were referring to in your previous reviews,
when we hot unplug a device:

"Device 'foo.N' does not have a release() function, it is broken and 
must be fixed."

See next patch that adds the required callback.

Regarding this particular get_device(), i have no clue why the
original author placed it here. I've looked over other bus 
implementations and didn't see something similar.

---
Best Regards, Laurentiu

Re: [PATCH 3/9] staging: fsl-mc: add device release callback

[Laurentiu Tudor]

On 02/03/2017 02:02 AM, Stuart Yoder wrote:
>
>> -Original Message-
>> From: upstream-release-boun...@linux.freescale.net [mailto:upstream-release-
>> boun...@linux.freescale.net] On Behalf Of laurentiu.tu...@nxp.com
>> Sent: Wednesday, February 01, 2017 5:43 AM
>> To: gre...@linuxfoundation.org
>> Cc: de...@driverdev.osuosl.org; a...@arndb.de; Ruxandra Ioana Radulescu 
>> <ruxandra.radule...@nxp.com>;
>> Roy Pledge <roy.ple...@nxp.com>; linux-kernel@vger.kernel.org; 
>> ag...@suse.de; Catalin Horghidan
>> <catalin.horghi...@nxp.com>; Leo Li <leoyang...@nxp.com>; Stuart Yoder 
>> <stuart.yo...@nxp.com>;
>> Laurentiu Tudor <laurentiu.tu...@nxp.com>
>> Subject: [upstream-release] [PATCH 3/9] staging: fsl-mc: add device release 
>> callback
>>
>> From: Laurentiu Tudor <laurentiu.tu...@nxp.com>
>>
>> When hot unplugging a mc-bus device the kernel displays
>> this pertinent message, followed by a stack dump:
>>      "Device 'foo.N' does not have a release() function,
>>   it is broken and must be fixed."
>> Add the required callback to fix.
>>
>> Signed-off-by: Laurentiu Tudor <laurentiu.tu...@nxp.com>
>> ---
>>   drivers/staging/fsl-mc/bus/fsl-mc-bus.c | 17 +
>>   1 file changed, 17 insertions(+)
>>
>> diff --git a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c 
>> b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
>> index 7c6a43b..6601bde 100644
>> --- a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
>> +++ b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c
>> @@ -419,6 +419,22 @@ bool fsl_mc_is_root_dprc(struct device *dev)
>>  return dev == root_dprc_dev;
>>   }
>>
>> +static void fsl_mc_device_release(struct device *dev)
>> +{
>> +struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
>> +struct fsl_mc_bus *mc_bus = NULL;
>> +
>> +kfree(mc_dev->regions);
>> +
>> +if (strcmp(mc_dev->obj_desc.type, "dprc") == 0)
>> +mc_bus = to_fsl_mc_bus(mc_dev);
>> +
>> +if (mc_bus)
>> +devm_kfree(mc_dev->dev.parent, mc_bus);
>> +else
>> +kmem_cache_free(mc_dev_cache, mc_dev);
>> +}
>> +
>>   /**
>>* Add a newly discovered fsl-mc device to be visible in Linux
>>*/
>> @@ -460,6 +476,7 @@ int fsl_mc_device_add(struct dprc_obj_desc *obj_desc,
>>  device_initialize(_dev->dev);
>>  mc_dev->dev.parent = parent_dev;
>>  mc_dev->dev.bus = _mc_bus_type;
>> +mc_dev->dev.release = fsl_mc_device_release;
>>  dev_set_name(_dev->dev, "%s.%d", obj_desc->type, obj_desc->id);
>>
>>  if (strcmp(obj_desc->type, "dprc") == 0) {
>> --
>
> With this patch applied, you still have this:
>
> void fsl_mc_device_remove(struct fsl_mc_device *mc_dev)
> {
>  struct fsl_mc_bus *mc_bus = NULL;
>
>  kfree(mc_dev->regions);
>
>  /*
>   * The device-specific remove callback will get invoked by 
> device_del()
>   */
>  device_del(_dev->dev);
>  put_device(_dev->dev);
>
>  if (strcmp(mc_dev->obj_desc.type, "dprc") == 0)
>  mc_bus = to_fsl_mc_bus(mc_dev);
>
>  if (mc_bus)
>  devm_kfree(mc_dev->dev.parent, mc_bus);
>  else
>  kmem_cache_free(mc_dev_cache, mc_dev);
> }
>
> ...i.e. you are doing the same thing in 2 places.  You
> need to remove the kfree/devm_kfree/ kmem_cache_free,
> here, no?
>

Right, thanks for spotting. I started working on a v2
of the series.

---
Best Regards, Laurentiu

Re: [PATCH] staging: fsl-mc: Add missing header

[Laurentiu Tudor]

On 02/13/2017 06:40 PM, Bogdan Purcareata wrote:
> Compiling the fsl-mc bus driver will yield a couple of static analysis
> errors:
> warning: symbol 'fsl_mc_msi_domain_alloc_irqs' was not declared
> warning: symbol 'fsl_mc_msi_domain_free_irqs' was not declared.
> warning: symbol 'its_fsl_mc_msi_init' was not declared.
> warning: symbol 'its_fsl_mc_msi_cleanup' was not declared.
>
> Since these are properly declared, but the header is not included, add
> it in the source files. This way the symbol is properly exported.
>
> Signed-off-by: Bogdan Purcareata <bogdan.purcare...@gmail.com>
> ---
> Sent against staging-testing.
>
>   drivers/staging/fsl-mc/bus/fsl-mc-msi.c| 1 +
>   drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c | 1 +
>   2 files changed, 2 insertions(+)
>
> diff --git a/drivers/staging/fsl-mc/bus/fsl-mc-msi.c 
> b/drivers/staging/fsl-mc/bus/fsl-mc-msi.c
> index 7975c6e..b8b2c86 100644
> --- a/drivers/staging/fsl-mc/bus/fsl-mc-msi.c
> +++ b/drivers/staging/fsl-mc/bus/fsl-mc-msi.c
> @@ -17,6 +17,7 @@
>   #include 
>   #include 
>   #include "../include/mc-bus.h"
> +#include "fsl-mc-private.h"
>
>   /*
>* Generate a unique ID identifying the interrupt (only used within the MSI
> diff --git a/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c 
> b/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
> index 0e2c1b5..87e4471 100644
> --- a/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
> +++ b/drivers/staging/fsl-mc/bus/irq-gic-v3-its-fsl-mc-msi.c
> @@ -17,6 +17,7 @@
>   #include 
>   #include 
>   #include "../include/mc-bus.h"
> +#include "fsl-mc-private.h"
>
>   static struct irq_chip its_msi_irq_chip = {
>   .name = "ITS-fMSI",
>

Acked-by: Laurentiu Tudor <laurentiu.tu...@nxp.com>

---
Best Regards, Laurentiu

Re: [PATCH] staging: fsl-mc: fix warning in DT ranges parser

[Laurentiu Tudor]

Hi Arnd,

On 02/27/2017 10:42 PM, Arnd Bergmann wrote:
> The fsl-mc-bus driver in staging contains a copy of the standard
> 'ranges' property parsing algorithm with a hack to treat a missing
> property the same way as an empty one. This code produces false-positive
> warnings for me in an allmodconfig build:
>
> drivers/staging/fsl-mc/bus/fsl-mc-bus.c: In function 'fsl_mc_bus_probe':
> drivers/staging/fsl-mc/bus/fsl-mc-bus.c:645:6: error: 'mc_size_cells' may be 
> used uninitialized in this function [-Werror=maybe-uninitialized]
> drivers/staging/fsl-mc/bus/fsl-mc-bus.c:682:8: error: 'mc_addr_cells' may be 
> used uninitialized in this function [-Werror=maybe-uninitialized]
> drivers/staging/fsl-mc/bus/fsl-mc-bus.c:644:6: note: 'mc_addr_cells' was 
> declared here
> drivers/staging/fsl-mc/bus/fsl-mc-bus.c:684:8: error: 'paddr_cells' may be 
> used uninitialized in this function [-Werror=maybe-uninitialized]
> drivers/staging/fsl-mc/bus/fsl-mc-bus.c:643:6: note: 'paddr_cells' was 
> declared here
>
> To avoid the warnings, I'm simplifying the argument handling to pass
> the number of valid ranges in the property as the function return code
> rather than passing it by reference. With this change, gcc can see that
> we don't evaluate the cell numbers for an missing ranges property.
>
> Signed-off-by: Arnd Bergmann <a...@arndb.de>

Looks good to me, i've tested it and did not see any issues, so here's an:

Acked-by: Laurentiu Tudor <laurentiu.tu...@nxp.com>

---
Thanks & Best Regards, Laurentiu

[PATCH] Documentation: kasan: arm64 has kasan support too

[Laurentiu Tudor]

Mention that arm64 also has kasan support.

Signed-off-by: Laurentiu Tudor <laurentiu.tu...@nxp.com>
---
 Documentation/kasan.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/kasan.txt b/Documentation/kasan.txt
index 7dd95b3..c156934 100644
--- a/Documentation/kasan.txt
+++ b/Documentation/kasan.txt
@@ -12,7 +12,7 @@ KASAN uses compile-time instrumentation for checking every 
memory access,
 therefore you will need a GCC version 4.9.2 or later. GCC 5.0 or later is
 required for detection of out-of-bounds accesses to stack or global variables.
 
-Currently KASAN is supported only for x86_64 architecture.
+Currently KASAN is supported only for x86_64 and arm64 architectures.
 
 1. Usage
 
-- 
1.8.3.1

Re: [PATCH v3 4/9] bus: fsl-mc: dpio: add frame descriptor and scatter/gather APIs

[Laurentiu Tudor]

On 12/05/2016 10:52 PM, Dan Carpenter wrote:
> On Fri, Dec 02, 2016 at 12:12:14PM +0000, Laurentiu Tudor wrote:
>>> +static inline bool dpaa2_sg_is_final(const struct dpaa2_sg_entry *sg)
>>> +{
>>> +   return !!(le16_to_cpu(sg->format_offset) >> SG_FINAL_FLAG_SHIFT);
>>
>> In other places in this file we don't use the '!!' to convert to bool. Maybe 
>> we should drop it here too.
> 
> I like the explicit "!!".  I think it makes the code more obvious since
> all the information is on one line.
> 

That's fine too, as long as we're doing it consistently in all the places.

---
Best Regards, Laurentiu

Re: [PATCH v3 5/9] bus: fsl-mc: dpio: add global dpaa2 definitions

[Laurentiu Tudor]

On 12/02/2016 12:41 AM, Stuart Yoder wrote:
> From: Roy Pledge 
> 
> Create header for global dpaa2 definitions.  Add definitions
> for dequeue results.
> 
> Signed-off-by: Roy Pledge 
> Signed-off-by: Stuart Yoder 
> ---
>  include/linux/fsl/dpaa2-global.h | 203 
> +++
>  1 file changed, 203 insertions(+)
>  create mode 100644 include/linux/fsl/dpaa2-global.h
> 
> diff --git a/include/linux/fsl/dpaa2-global.h 
> b/include/linux/fsl/dpaa2-global.h
> new file mode 100644
> index 000..3ee3f29
> --- /dev/null
> +++ b/include/linux/fsl/dpaa2-global.h
> @@ -0,0 +1,203 @@
> +/*
> + * Copyright 2014-2016 Freescale Semiconductor Inc.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are 
> met:
> + * * Redistributions of source code must retain the above copyright
> + *   notice, this list of conditions and the following disclaimer.
> + * * Redistributions in binary form must reproduce the above copyright
> + *   notice, this list of conditions and the following disclaimer in the
> + *   documentation and/or other materials provided with the distribution.
> + * * Neither the name of Freescale Semiconductor nor the
> + *   names of its contributors may be used to endorse or promote products
> + *   derived from this software without specific prior written 
> permission.
> + *
> + * ALTERNATIVELY, this software may be distributed under the terms of the
> + * GNU General Public License ("GPL") as published by the Free Software
> + * Foundation, either version 2 of that License or (at your option) any
> + * later version.
> + *
> + * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor ``AS IS'' AND ANY
> + * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
> + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> + * DISCLAIMED. IN NO EVENT SHALL Freescale Semiconductor BE LIABLE FOR ANY
> + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR 
> SERVICES;
> + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED 
> AND
> + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF 
> THIS
> + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +#ifndef __FSL_DPAA2_GLOBAL_H
> +#define __FSL_DPAA2_GLOBAL_H
> +
> +#include 
> +#include 
> +#include 
> +
> +struct dpaa2_dq {
> + union {
> + struct common {
> + u8 verb;
> + u8 reserved[63];
> + } common;
> + struct dq {
> + u8 verb;
> + u8 stat;
> + __le16 seqnum;
> + __le16 oprid;
> + u8 reserved;
> + u8 tok;
> + __le32 fqid;
> + u32 reserved2;
> + __le32 fq_byte_cnt;
> + __le32 fq_frm_cnt;
> + __le64 fqd_ctx;
> + u8 fd[32];
> + } dq;
> + struct scn {
> + u8 verb;
> + u8 stat;
> + u8 state;
> + u8 reserved;
> + __le32 rid_tok;
> + __le64 ctx;
> + } scn;
> + };
> +};
> +
> +

Extra blank line.

> +/* Parsing frame dequeue results */
> +/* FQ empty */
> +#define DPAA2_DQ_STAT_FQEMPTY   0x80
> +/* FQ held active */
> +#define DPAA2_DQ_STAT_HELDACTIVE0x40
> +/* FQ force eligible */
> +#define DPAA2_DQ_STAT_FORCEELIGIBLE 0x20
> +/* valid frame */
> +#define DPAA2_DQ_STAT_VALIDFRAME0x10
> +/* FQ ODP enable */
> +#define DPAA2_DQ_STAT_ODPVALID  0x04
> +/* volatile dequeue */
> +#define DPAA2_DQ_STAT_VOLATILE  0x02
> +/* volatile dequeue command is expired */
> +#define DPAA2_DQ_STAT_EXPIRED   0x01
> +
> +#define DQ_FQID_MASK 0x00FF
> +#define DQ_FRAME_COUNT_MASK 0x00FF

We should have these 2 macro values aligned too.

> +/**
> + * dpaa2_dq_flags() - Get the stat field of dequeue response
> + * @dq: the dequeue result.
> + */
> +static inline u32 dpaa2_dq_flags(const struct dpaa2_dq *dq)
> +{
> + return dq->dq.stat;
> +}
> +
> +/**
> + * dpaa2_dq_is_pull() - Check whether the dq response is from a pull
> + *  command.
> + * @dq: the dequeue result
> + *
> + * Return 1 for volatile(pull) dequeue, 0 for static dequeue.
> + */
> +static inline int dpaa2_dq_is_pull(const struct dpaa2_dq *dq)
> +{
> + return (int)(dpaa2_dq_flags(dq) & DPAA2_DQ_STAT_VOLATILE);
> +}
> +
> +/**
> + * dpaa2_dq_is_pull_complete() - Check whether the pull command is 

Re: [PATCH v3 6/9] bus: fsl-mc: dpio: add QBMan portal APIs for DPAA2

[Laurentiu Tudor]

On 12/02/2016 12:41 AM, Stuart Yoder wrote:
> From: Roy Pledge 
> 
> Add QBman APIs for frame queue and buffer pool operations.
> 
> Signed-off-by: Roy Pledge 
> Signed-off-by: Haiying Wang 
> Signed-off-by: Stuart Yoder 
> ---
> 
> Notes:
> -v3
>-replace hardcoded dequeue token with a #define and check that
> token when checking for a new result (bug fix suggested by
> Ioana Radulescu)
> -v2
>-fix bug in buffer release command, by setting bpid field
>-handle error (NULL) return value from qbman_swp_mc_complete()
>-error message cleanup
>-fix bug in sending management commands where the verb was
> properly initialized
> 
>  drivers/bus/fsl-mc/dpio/Makefile   |2 +-
>  drivers/bus/fsl-mc/dpio/qbman-portal.c | 1028 
> 
>  drivers/bus/fsl-mc/dpio/qbman-portal.h |  464 ++
>  3 files changed, 1493 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/bus/fsl-mc/dpio/qbman-portal.c
>  create mode 100644 drivers/bus/fsl-mc/dpio/qbman-portal.h
> 
> diff --git a/drivers/bus/fsl-mc/dpio/Makefile 
> b/drivers/bus/fsl-mc/dpio/Makefile
> index 128befc..6588498 100644
> --- a/drivers/bus/fsl-mc/dpio/Makefile
> +++ b/drivers/bus/fsl-mc/dpio/Makefile
> @@ -6,4 +6,4 @@ subdir-ccflags-y := -Werror
>  
>  obj-$(CONFIG_FSL_MC_DPIO) += fsl-mc-dpio.o
>  
> -fsl-mc-dpio-objs := dpio.o
> +fsl-mc-dpio-objs := dpio.o qbman-portal.o
> diff --git a/drivers/bus/fsl-mc/dpio/qbman-portal.c 
> b/drivers/bus/fsl-mc/dpio/qbman-portal.c
> new file mode 100644
> index 000..bbc032c
> --- /dev/null
> +++ b/drivers/bus/fsl-mc/dpio/qbman-portal.c
> @@ -0,0 +1,1028 @@
> +/*
> + * Copyright (C) 2014 Freescale Semiconductor, Inc.

In previous patches the copyright years are 2014 - 2016. Maybe we should
do the same here too.

> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are 
> met:
> + * * Redistributions of source code must retain the above copyright
> + *   notice, this list of conditions and the following disclaimer.
> + * * Redistributions in binary form must reproduce the above copyright
> + *   notice, this list of conditions and the following disclaimer in the
> + *   documentation and/or other materials provided with the distribution.
> + * * Neither the name of Freescale Semiconductor nor the
> + *   names of its contributors may be used to endorse or promote products
> + *   derived from this software without specific prior written 
> permission.
> + *
> + * ALTERNATIVELY, this software may be distributed under the terms of the
> + * GNU General Public License ("GPL") as published by the Free Software
> + * Foundation, either version 2 of that License or (at your option) any
> + * later version.
> + *
> + * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor ``AS IS'' AND ANY
> + * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
> + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> + * DISCLAIMED. IN NO EVENT SHALL Freescale Semiconductor BE LIABLE FOR ANY
> + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR 
> SERVICES;
> + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED 
> AND
> + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF 
> THIS
> + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "qbman-portal.h"
> +
> +#define QMAN_REV_4000   0x0400
> +#define QMAN_REV_4100   0x0401
> +#define QMAN_REV_4101   0x04010001
> +
> +/* All QBMan command and result structures use this "valid bit" encoding */
> +#define QB_VALID_BIT ((u32)0x80)
> +
> +/* QBMan portal management command codes */
> +#define QBMAN_MC_ACQUIRE   0x30
> +#define QBMAN_WQCHAN_CONFIGURE 0x46
> +
> +/* CINH register offsets */
> +#define QBMAN_CINH_SWP_EQAR0x8c0
> +#define QBMAN_CINH_SWP_DQPI0xa00
> +#define QBMAN_CINH_SWP_DCAP0xac0
> +#define QBMAN_CINH_SWP_SDQCR   0xb00
> +#define QBMAN_CINH_SWP_RAR 0xcc0
> +#define QBMAN_CINH_SWP_ISR 0xe00
> +#define QBMAN_CINH_SWP_IER 0xe40
> +#define QBMAN_CINH_SWP_ISDR0xe80
> +#define QBMAN_CINH_SWP_IIR 0xec0
> +
> +/* CENA register offsets */
> +#define QBMAN_CENA_SWP_EQCR(n) (0x000 + ((u32)(n) << 6))
> +#define QBMAN_CENA_SWP_DQRR(n) (0x200 + ((u32)(n) << 6))
> +#define QBMAN_CENA_SWP_RCR(n)  (0x400 + ((u32)(n) << 6))
> +#define QBMAN_CENA_SWP_CR  0x600
> +#define QBMAN_CENA_SWP_RR(vb)  (0x700 + ((u32)(vb) >> 1))
> +#define QBMAN_CENA_SWP_VDQCR   0x780
> +
> +/* Reverse 

Re: [PATCH v3 7/9] bus: fsl-mc: dpio: add the DPAA2 DPIO service interface

[Laurentiu Tudor]

On 12/02/2016 12:41 AM, Stuart Yoder wrote:
> From: Roy Pledge 
> 
> The DPIO service interface handles initialization of DPIO objects
> and exports APIs to be used by other DPAA2 object drivers to perform
> queuing and buffer management related operations.  The service allows
> registration of callbacks when frames or notifications are received.
> 
> Signed-off-by: Roy Pledge 
> Signed-off-by: Haiying Wang 
> Signed-off-by: Stuart Yoder 
> ---
> 
> Notes:
> -v3
>-zero memory allocated for a dpio store (bug fix suggested
> by Ioana Radulescu)
> -v2
>-use service_select_by_cpu() for re-arming DPIO interrupts
>-replace use of NR_CPUS with num_possible_cpus()
> 
>  drivers/bus/fsl-mc/dpio/Makefile   |   2 +-
>  drivers/bus/fsl-mc/dpio/dpio-service.c | 614 
> +
>  include/linux/fsl/dpaa2-io.h   | 138 
>  3 files changed, 753 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/bus/fsl-mc/dpio/dpio-service.c
>  create mode 100644 include/linux/fsl/dpaa2-io.h
> 
> diff --git a/drivers/bus/fsl-mc/dpio/Makefile 
> b/drivers/bus/fsl-mc/dpio/Makefile
> index 6588498..0778da7 100644
> --- a/drivers/bus/fsl-mc/dpio/Makefile
> +++ b/drivers/bus/fsl-mc/dpio/Makefile
> @@ -6,4 +6,4 @@ subdir-ccflags-y := -Werror
>  
>  obj-$(CONFIG_FSL_MC_DPIO) += fsl-mc-dpio.o
>  
> -fsl-mc-dpio-objs := dpio.o qbman-portal.o
> +fsl-mc-dpio-objs := dpio.o qbman-portal.o dpio-service.o
> diff --git a/drivers/bus/fsl-mc/dpio/dpio-service.c 
> b/drivers/bus/fsl-mc/dpio/dpio-service.c
> new file mode 100644
> index 000..01f0293
> --- /dev/null
> +++ b/drivers/bus/fsl-mc/dpio/dpio-service.c
> @@ -0,0 +1,614 @@
> +/*
> + * Copyright 2014-2016 Freescale Semiconductor Inc.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are 
> met:
> + * * Redistributions of source code must retain the above copyright
> + *notice, this list of conditions and the following disclaimer.
> + * * Redistributions in binary form must reproduce the above copyright
> + *notice, this list of conditions and the following disclaimer in the
> + *documentation and/or other materials provided with the distribution.
> + * * Neither the name of Freescale Semiconductor nor the
> + *names of its contributors may be used to endorse or promote products
> + *derived from this software without specific prior written permission.
> + *
> + *
> + * ALTERNATIVELY, this software may be distributed under the terms of the
> + * GNU General Public License ("GPL") as published by the Free Software
> + * Foundation, either version 2 of that License or (at your option) any
> + * later version.
> + *
> + * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor ``AS IS'' AND ANY
> + * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
> + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> + * DISCLAIMED. IN NO EVENT SHALL Freescale Semiconductor BE LIABLE FOR ANY
> + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR 
> SERVICES;
> + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED 
> AND
> + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF 
> THIS
> + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "dpio.h"
> +#include "qbman-portal.h"
> +
> +struct dpaa2_io {
> + atomic_t refs;
> + struct dpaa2_io_desc dpio_desc;
> + struct qbman_swp_desc swp_desc;
> + struct qbman_swp *swp;
> + struct list_head node;
> + spinlock_t lock_mgmt_cmd;
> + spinlock_t lock_notifications;
> + struct list_head notifications;
> +};
> +
> +struct dpaa2_io_store {
> + unsigned int max;
> + dma_addr_t paddr;
> + struct dpaa2_dq *vaddr;
> + void *alloced_addr;/* unaligned value from kmalloc() */
> + unsigned int idx;  /* position of the next-to-be-returned entry */
> + struct qbman_swp *swp; /* portal used to issue VDQCR */
> + struct device *dev;/* device used for DMA mapping */
> +};
> +
> +/* keep a per cpu array of DPIOs for fast access */
> +static struct dpaa2_io *dpio_by_cpu[NR_CPUS];
> +static struct list_head dpio_list = LIST_HEAD_INIT(dpio_list);
> +static DEFINE_SPINLOCK(dpio_list_lock);
> +
> +static inline struct dpaa2_io *service_select_by_cpu(struct dpaa2_io *d,
> +  int cpu)
> +{
> + if (d)
> + return d;
> +
> + if (unlikely(cpu >= 

Re: [PATCH v3 4/9] bus: fsl-mc: dpio: add frame descriptor and scatter/gather APIs

[Laurentiu Tudor]

Some more bits and pieces inside.

---
Best Regards, Laurentiu

On 12/02/2016 12:41 AM, Stuart Yoder wrote:
> From: Roy Pledge 
> 
> Add global definitions for DPAA2 frame descriptors and scatter
> gather entries.
> 
> Signed-off-by: Roy Pledge 
> Signed-off-by: Stuart Yoder 
> ---
> 
> Notes:
> -v3
>-no changes
> -v2
>-added setter/getter for the FD ctrl field
>-corrected comment for SG format_offset field description
>-added support for short length field in FD
> 
>  include/linux/fsl/dpaa2-fd.h | 448 
> +++
>  1 file changed, 448 insertions(+)
>  create mode 100644 include/linux/fsl/dpaa2-fd.h
> 
> diff --git a/include/linux/fsl/dpaa2-fd.h b/include/linux/fsl/dpaa2-fd.h
> new file mode 100644
> index 000..182c8f4
> --- /dev/null
> +++ b/include/linux/fsl/dpaa2-fd.h
> @@ -0,0 +1,448 @@
> +/*
> + * Copyright 2014-2016 Freescale Semiconductor Inc.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are 
> met:
> + * * Redistributions of source code must retain the above copyright
> + *   notice, this list of conditions and the following disclaimer.
> + * * Redistributions in binary form must reproduce the above copyright
> + *   notice, this list of conditions and the following disclaimer in the
> + *   documentation and/or other materials provided with the distribution.
> + * * Neither the name of Freescale Semiconductor nor the
> + *   names of its contributors may be used to endorse or promote products
> + *   derived from this software without specific prior written 
> permission.
> + *
> + * ALTERNATIVELY, this software may be distributed under the terms of the
> + * GNU General Public License ("GPL") as published by the Free Software
> + * Foundation, either version 2 of that License or (at your option) any
> + * later version.
> + *
> + * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor ``AS IS'' AND ANY
> + * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
> + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> + * DISCLAIMED. IN NO EVENT SHALL Freescale Semiconductor BE LIABLE FOR ANY
> + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR 
> SERVICES;
> + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED 
> AND
> + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF 
> THIS
> + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +#ifndef __FSL_DPAA2_FD_H
> +#define __FSL_DPAA2_FD_H
> +
> +#include 
> +
> +/**
> + * DOC: DPAA2 FD - Frame Descriptor APIs for DPAA2
> + *
> + * Frame Descriptors (FDs) are used to describe frame data in the DPAA2.
> + * Frames can be enqueued and dequeued to Frame Queues (FQs) which are 
> consumed
> + * by the various DPAA accelerators (WRIOP, SEC, PME, DCE)
> + *
> + * There are three types of frames: single, scatter gather, and frame lists.
> + *
> + * The set of APIs in this file must be used to create, manipulate and
> + * query Frame Descriptors.
> + */
> +
> +/**
> + * struct dpaa2_fd - Struct describing FDs
> + * @words: for easier/faster copying the whole FD structure
> + * @addr:  address in the FD
> + * @len:   length in the FD
> + * @bpid:  buffer pool ID
> + * @format_offset: format, offset, and short-length fields
> + * @frc:   frame context
> + * @ctrl:  control bits...including dd, sc, va, err, etc
> + * @flc:   flow context address
> + *
> + * This structure represents the basic Frame Descriptor used in the system.
> + */
> +struct dpaa2_fd {
> + union {
> + u32 words[8];
> + struct dpaa2_fd_simple {
> + __le64 addr;
> + __le32 len;
> + __le16 bpid;
> + __le16 format_offset;
> + __le32 frc;
> + __le32 ctrl;
> + __le64 flc;
> + } simple;
> + };
> +};
> +
> +#define FD_SHORT_LEN_FLAG_MASK 0x1
> +#define FD_SHORT_LEN_FLAG_SHIFT 14
> +#define FD_SHORT_LEN_MASK 0x1
> +#define FD_OFFSET_MASK 0x0FFF
> +#define FD_FORMAT_MASK 0x3
> +#define FD_FORMAT_SHIFT 12
> +#define SG_SHORT_LEN_FLAG_MASK 0x1
> +#define SG_SHORT_LEN_FLAG_SHIFT 14
> +#define SG_SHORT_LEN_MASK 0x1
> +#define SG_OFFSET_MASK 0x0FFF
> +#define SG_FORMAT_MASK 0x3
> +#define SG_FORMAT_SHIFT 12
> +#define SG_BPID_MASK 0x3FFF
> +#define SG_FINAL_FLAG_MASK 0x1
> +#define SG_FINAL_FLAG_SHIFT 15

We should align the values of these macros as we do in other places.

> +enum dpaa2_fd_format 

Re: [PATCH v3 3/9] bus: fsl-mc: dpio: add APIs for DPIO objects

[Laurentiu Tudor]

Couple of nits inline.

---
Best Regards, Laurentiu

On 12/02/2016 12:41 AM, Stuart Yoder wrote:
> From: Ioana Radulescu 
> 
> Add the command build/parse APIs for operating on DPIO objects through
> the DPAA2 Management Complex.
> 
> Signed-off-by: Ioana Radulescu 
> Signed-off-by: Roy Pledge 
> Signed-off-by: Stuart Yoder 
> ---
> 
> Notes:
> -v3
>   -no changes
> -v2
>   -removed unused structs and defines
> 
>  drivers/bus/fsl-mc/Kconfig |  10 ++
>  drivers/bus/fsl-mc/Makefile|   3 +
>  drivers/bus/fsl-mc/dpio/Makefile   |   9 ++
>  drivers/bus/fsl-mc/dpio/dpio-cmd.h |  75 
>  drivers/bus/fsl-mc/dpio/dpio.c | 229 
> +
>  drivers/bus/fsl-mc/dpio/dpio.h | 108 +
>  6 files changed, 434 insertions(+)
>  create mode 100644 drivers/bus/fsl-mc/dpio/Makefile
>  create mode 100644 drivers/bus/fsl-mc/dpio/dpio-cmd.h
>  create mode 100644 drivers/bus/fsl-mc/dpio/dpio.c
>  create mode 100644 drivers/bus/fsl-mc/dpio/dpio.h
> 
> diff --git a/drivers/bus/fsl-mc/Kconfig b/drivers/bus/fsl-mc/Kconfig
> index 5c009ab..a10aaf0 100644
> --- a/drivers/bus/fsl-mc/Kconfig
> +++ b/drivers/bus/fsl-mc/Kconfig
> @@ -15,3 +15,13 @@ config FSL_MC_BUS
> architecture.  The fsl-mc bus driver handles discovery of
> DPAA2 objects (which are represented as Linux devices) and
> binding objects to drivers.
> +
> +config FSL_MC_DPIO
> +tristate "QorIQ DPAA2 DPIO driver"
> +depends on FSL_MC_BUS
> +help
> +   Driver for the DPAA2 DPIO object.  A DPIO provides queue and
> +   buffer management facilities for software to interact with
> +   other DPAA2 objects. This driver does not expose the DPIO
> +   objects individually, but groups them under a service layer
> +   API.
> diff --git a/drivers/bus/fsl-mc/Makefile b/drivers/bus/fsl-mc/Makefile
> index d56afee..d18df72 100644
> --- a/drivers/bus/fsl-mc/Makefile
> +++ b/drivers/bus/fsl-mc/Makefile
> @@ -17,3 +17,6 @@ mc-bus-driver-objs := fsl-mc-bus.o \
> fsl-mc-msi.o \
> dpmcp.o \
> dpbp.o
> +
> +# MC DPIO driver
> +obj-$(CONFIG_FSL_MC_DPIO) += dpio/
> diff --git a/drivers/bus/fsl-mc/dpio/Makefile 
> b/drivers/bus/fsl-mc/dpio/Makefile
> new file mode 100644
> index 000..128befc
> --- /dev/null
> +++ b/drivers/bus/fsl-mc/dpio/Makefile
> @@ -0,0 +1,9 @@
> +#
> +# QorIQ DPAA2 DPIO driver
> +#
> +
> +subdir-ccflags-y := -Werror
> +
> +obj-$(CONFIG_FSL_MC_DPIO) += fsl-mc-dpio.o
> +
> +fsl-mc-dpio-objs := dpio.o
> diff --git a/drivers/bus/fsl-mc/dpio/dpio-cmd.h 
> b/drivers/bus/fsl-mc/dpio/dpio-cmd.h
> new file mode 100644
> index 000..3464ed9
> --- /dev/null
> +++ b/drivers/bus/fsl-mc/dpio/dpio-cmd.h
> @@ -0,0 +1,75 @@
> +/*
> + * Copyright 2013-2016 Freescale Semiconductor Inc.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are 
> met:
> + * * Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * * Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + * * Neither the name of the above-listed copyright holders nor the
> + * names of any contributors may be used to endorse or promote products
> + * derived from this software without specific prior written permission.
> + *
> + * ALTERNATIVELY, this software may be distributed under the terms of the
> + * GNU General Public License ("GPL") as published by the Free Software
> + * Foundation, either version 2 of that License or (at your option) any
> + * later version.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS 
> IS"
> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDERS OR CONTRIBUTORS BE
> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> + * POSSIBILITY OF SUCH DAMAGE.
> + */
> +#ifndef _FSL_DPIO_CMD_H
> +#define _FSL_DPIO_CMD_H
> +
> +/* DPIO Version */
> +#define DPIO_VER_MAJOR   4
> +#define DPIO_VER_MINOR   2
> +
> 

Re: linux-next: build failure in the staging tree (Was: kisskb: FAILED linux-next/s390-allmodconfig/s390x Mon Jul 31, 17:24)

[Laurentiu Tudor]

Hi Stephen,

That's because the fsl-mc driver selects GENERIC_MSI_IRQ_DOMAIN and not 
all arches implement the support for the option. I can submit a patch 
that adds explicit dependencies on arches that it was build-tested (x86, 
arm, powerpc, all both 32 and 64 bits) similar to how it's done here 
[1]. Let me know if you're ok with this fix and i'll submit the fix to 
staging.

---
Thanks & Best Regards, Laurentiu

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/Kconfig#n28

On 07/31/2017 12:35 PM, Stephen Rothwell wrote:
> Hi all,
>
> On Mon, 31 Jul 2017 07:24:25 - nore...@ellerman.id.au wrote:
>>
>> FAILED linux-next/s390-allmodconfig/s390x Mon Jul 31, 17:24
>>
>> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fkisskb.ellerman.id.au%2Fkisskb%2Fbuildresult%2F13110910%2F=01%7C01%7Claurentiu.tudor%40nxp.com%7C0b58775ead154830f76f08d4d7f78d99%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0=A84bbMQe7VRz9OPK13PzaidBEfjm43Ik%2FbfCDJ89ZzM%3D=0
>>
>> Commit:   Add linux-next specific files for 20170731
>>742f220ee69c8102eabea45e45d92aa18721fab1
>> Compiler: s390x-linux-gcc (GCC) 4.6.3
>>
>> Possible errors
>> ---
>>
>> include/linux/msi.h:196:21: fatal error: asm/msi.h: No such file or directory
>> make[2]: *** [arch/s390/kernel/asm-offsets.s] Error 1
>> make[1]: *** [prepare0] Error 2
>> make: *** [sub-make] Error 2
>
> Caused by commit
>
>03274850279c ("staging: fsl-mc: allow the driver compile multi-arch")
>
> I will revert that commit tomorrow unless it if fixed in the mean time.
>

Re: linux-next: build failure in the staging tree (Was: kisskb: FAILED linux-next/s390-allmodconfig/s390x Mon Jul 31, 17:24)

[Laurentiu Tudor]

On 07/31/2017 06:58 PM, Greg KH wrote:
> On Mon, Jul 31, 2017 at 09:55:14AM +0000, Laurentiu Tudor wrote:
>> Hi Stephen,
>>
>> That's because the fsl-mc driver selects GENERIC_MSI_IRQ_DOMAIN and not
>> all arches implement the support for the option. I can submit a patch
>> that adds explicit dependencies on arches that it was build-tested (x86,
>> arm, powerpc, all both 32 and 64 bits) similar to how it's done here
>> [1]. Let me know if you're ok with this fix and i'll submit the fix to
>> staging.
>
> Ugh, you should not be selecting that option, but rather depending on
> the option, right?

All users in the kernel use "select", so i don't think so. An 
interesting use that adds explicit dependencies on architectures can be
seen here [1], in the generic code. I've proposed a patch [2] that does 
a similar thing for mc-bus. I think it's a good approach as it keeps 
things under control by explicitly specifying the architectures on which
the driver was compile-tested.

---
Best Regards, Laurentiu

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/Kconfig#n28
[2] https://patchwork.kernel.org/patch/9871861/

Re: [PATCH] staging: fsl-mc: fix fsl_mc_is_allocatable strcmps

[Laurentiu Tudor]

On 08/16/2017 03:06 PM, Dan Carpenter wrote:
> On Wed, Aug 16, 2017 at 12:44:51PM +0100, Colin King wrote:
>> From: Colin Ian King 
>>
>> The previous fix removed the equal to zero comparisons by the strcmps and
>> now the function always returns true. Fix this by adding in the missing
>> logical negation operators.
>>
>> Detected by CoverityScan, CID#1452267 ("Constant expression result")
>>
>> Fixes: b93ad9a067e1 ("staging: fsl-mc: be consistent when checking strcmp() 
>> return")

Thanks Colin (and Coverity) for catching this!

> Ugh...  I did review the original patch at all.  Sorry.

As a side note, funny how i got the patch description right but not the 
actual patch. :-)

> It's better to use "== 0" because it's idiomatic.

Agree, plus this approach would be consistent with the rest of the 
driver (except one place in drivers/staging/fsl-mc/bus/dprc-driver.c +32)

---
Best Regards, Laurentiu

Re: [PATCH 6/7] staging: fsl-mc: rewrite mc command send/receive to work on 32-bits

[Laurentiu Tudor]

Hi Robin,

On 07/17/2017 04:43 PM, Robin Murphy wrote:
> On 17/07/17 14:26, laurentiu.tu...@nxp.com wrote:
>> From: Laurentiu Tudor <laurentiu.tu...@nxp.com>
>>
>> Split the 64-bit accesses in 32-bit accesses because there's no real
>> constrain in MC to do only atomic 64-bit. There's only one place
>> where ordering is important: when writing the MC command header the
>> first 32-bit part of the header must be written last.
>> We do this switch in order to allow compiling the driver on 32-bit.
>
> #include 
>
> Then you can just use writeq()/writeq_relaxed()/readq_relaxed() on
> 32-bit platforms as well.
>

Thanks, didn't knew of the header. This should take care of the 
compilation errors i was seeing. There's one problem though: these 
handle byte-ordering and i need raw accessors. :-(

---
Best Regards, Laurentiu

>> Signed-off-by: Laurentiu Tudor <laurentiu.tu...@nxp.com>
>> ---
>>   drivers/staging/fsl-mc/bus/mc-sys.c | 31 ---
>>   1 file changed, 12 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/staging/fsl-mc/bus/mc-sys.c 
>> b/drivers/staging/fsl-mc/bus/mc-sys.c
>> index 195d9f3..dd2828e 100644
>> --- a/drivers/staging/fsl-mc/bus/mc-sys.c
>> +++ b/drivers/staging/fsl-mc/bus/mc-sys.c
>> @@ -124,14 +124,15 @@ static inline void mc_write_command(struct mc_command 
>> __iomem *portal,
>>   {
>>  int i;
>>
>> -/* copy command parameters into the portal */
>> -for (i = 0; i < MC_CMD_NUM_OF_PARAMS; i++)
>> -__raw_writeq(cmd->params[i], >params[i]);
>> -/* ensure command params are committed before submitting it */
>> -wmb();
>> -
>> -/* submit the command by writing the header */
>> -__raw_writeq(cmd->header, >header);
>> +/*
>> + * copy command parameters into the portal. Final write
>> + * triggers the submission of the command.
>> + */
>> +for (i = sizeof(struct mc_command) / sizeof(u32) - 1; i >= 0; i--) {
>> +__raw_writel(((u32 *)cmd)[i], &((u32 *)portal)[i]);
>> +/* ensure command params are committed before submitting it */
>> +wmb();
>> +}
>>   }
>>
>>   /**
>> @@ -148,19 +149,11 @@ static inline enum mc_cmd_status 
>> mc_read_response(struct mc_command __iomem *
>>struct mc_command *resp)
>>   {
>>  int i;
>> -enum mc_cmd_status status;
>> -
>> -/* Copy command response header from MC portal: */
>> -resp->header = __raw_readq(>header);
>> -status = mc_cmd_hdr_read_status(resp);
>> -if (status != MC_CMD_STATUS_OK)
>> -return status;
>>
>> -/* Copy command response data from MC portal: */
>> -for (i = 0; i < MC_CMD_NUM_OF_PARAMS; i++)
>> -resp->params[i] = __raw_readq(>params[i]);
>> +for (i = 0; i < sizeof(struct mc_command) / sizeof(u32); i++)
>> +((u32 *)resp)[i] = __raw_readl(&((u32 *)portal)[i]);
>>
>> -return status;
>> +return mc_cmd_hdr_read_status(resp);
>>   }
>>
>>   /**
>>
>

Re: [PATCH 2/7] staging: fsl-mc: use generic memory barriers

[Laurentiu Tudor]

On 07/17/2017 04:38 PM, Robin Murphy wrote:
> On 17/07/17 14:26, laurentiu.tu...@nxp.com wrote:
>> From: Laurentiu Tudor <laurentiu.tu...@nxp.com>
>>
>> No need to use arch-specific memory barriers; switch to using generic
>> ones. The rmb()s were useless so drop them.
>>
>> Signed-off-by: Laurentiu Tudor <laurentiu.tu...@nxp.com>
>> ---
>>   drivers/staging/fsl-mc/bus/mc-sys.c | 6 ++
>>   1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/staging/fsl-mc/bus/mc-sys.c 
>> b/drivers/staging/fsl-mc/bus/mc-sys.c
>> index a1704c3..012abd5 100644
>> --- a/drivers/staging/fsl-mc/bus/mc-sys.c
>> +++ b/drivers/staging/fsl-mc/bus/mc-sys.c
>> @@ -127,7 +127,8 @@ static inline void mc_write_command(struct mc_command 
>> __iomem *portal,
>>  /* copy command parameters into the portal */
>>  for (i = 0; i < MC_CMD_NUM_OF_PARAMS; i++)
>>  __raw_writeq(cmd->params[i], >params[i]);
>> -__iowmb();
>> +/* ensure command params are committed before submitting it */
>> +wmb();
>>
>>  /* submit the command by writing the header */
>>  __raw_writeq(cmd->header, >header);
>
> AFAICS, just using writeq() here should ensure sufficient order against
> the previous iomem accessors, without the need for explicit barriers.

Endianess is handled in the callers, this function should leave the raw 
data unchanged. So the raw version was chosen on purpose.

> Also, note that the __raw_*() variants aren't endian-safe, so consider
> updating things to *_relaxed() where ordering doesn't matter.
>

That's what i tried in the first place but encountered compilation 
errors on 32-bit powerpc & 32-bit x86 having to do with writeq/readq 
variants not being available (IIRC). So that's why i switched to the 
32-bit variants in the end.

---
Thanks & Best Regards, Laurentiu

Re: [PATCH 6/7] staging: fsl-mc: rewrite mc command send/receive to work on 32-bits

[Laurentiu Tudor]

Hi Arnd,

On 07/17/2017 04:45 PM, Arnd Bergmann wrote:
> On Mon, Jul 17, 2017 at 3:26 PM,  <laurentiu.tu...@nxp.com> wrote:
>> From: Laurentiu Tudor <laurentiu.tu...@nxp.com>
>>
>> Split the 64-bit accesses in 32-bit accesses because there's no real
>> constrain in MC to do only atomic 64-bit. There's only one place
>> where ordering is important: when writing the MC command header the
>> first 32-bit part of the header must be written last.
>> We do this switch in order to allow compiling the driver on 32-bit.
>>
>> Signed-off-by: Laurentiu Tudor <laurentiu.tu...@nxp.com>
>> ---
>>   drivers/staging/fsl-mc/bus/mc-sys.c | 31 ---
>>   1 file changed, 12 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/staging/fsl-mc/bus/mc-sys.c 
>> b/drivers/staging/fsl-mc/bus/mc-sys.c
>> index 195d9f3..dd2828e 100644
>> --- a/drivers/staging/fsl-mc/bus/mc-sys.c
>> +++ b/drivers/staging/fsl-mc/bus/mc-sys.c
>> @@ -124,14 +124,15 @@ static inline void mc_write_command(struct mc_command 
>> __iomem *portal,
>>   {
>>  int i;
>>
>> -   /* copy command parameters into the portal */
>> -   for (i = 0; i < MC_CMD_NUM_OF_PARAMS; i++)
>> -   __raw_writeq(cmd->params[i], >params[i]);
>> -   /* ensure command params are committed before submitting it */
>> -   wmb();
>> -
>> -   /* submit the command by writing the header */
>> -   __raw_writeq(cmd->header, >header);
>> +   /*
>> +* copy command parameters into the portal. Final write
>> +* triggers the submission of the command.
>> +*/
>> +   for (i = sizeof(struct mc_command) / sizeof(u32) - 1; i >= 0; i--) {
>> +   __raw_writel(((u32 *)cmd)[i], &((u32 *)portal)[i]);
>> +   /* ensure command params are committed before submitting it 
>> */
>> +   wmb();
>> +   }
>>   }
>
> What is the byte order requirement on this buffer?

Endianness is handled by the callers so this function must leave
the binary blob intact.

> If this is a byte string
> rather than individual registers, you should probably just use
> memcpy_toio()

It's a header followed by an opaque command. The protocol for queueing a 
command says that the first 32-bit half of the header must be written 
last, this triggering the command handling in the MC.

> but if these are separate registers, then using the
> __raw_* accessors is still wrong, at least on kernels that have a
> different byteorder from the hardware.

As mentioned above, endianness is handled by the caller. This function
takes raw data and must leave it unchanged.

> Also, are you sure that adding those six extra barriers has no
> performance impact?

This is a slow interface used in slow paths, so i don't think those 
extra barriers will have any performance impact.

---
Thanks & Best Regards, Laurentiu

Re: [PATCH v2 6/8] staging: fsl-mc: don't use raw device io functions

[Laurentiu Tudor]

Hi Arnd,

On 07/18/2017 05:18 PM, Arnd Bergmann wrote:
> On Tue, Jul 18, 2017 at 3:37 PM,  <laurentiu.tu...@nxp.com> wrote:
>> From: Laurentiu Tudor <laurentiu.tu...@nxp.com>
>>
>> As raw device io functions are not portable and don't handle byte-order
>> (triggering suspicion that endianness isn't handled well) switch to
>> using the standard api.
>> Since MC expects LE byte-order and the upper layers already take care
>> of that, we need to trick the device io api by doing a LE -> CPU
>> conversion just before calling it. This way, the CPU -> LE conversion
>> done in the api puts the data back in the right byte-order. Obviously,
>> for reads the extra step is mirrored: there's a CPU -> LE conversion
>> following the API call.
>>
>> Signed-off-by: Laurentiu Tudor <laurentiu.tu...@nxp.com>
>> ---
>> Notes:
>>  -v2
>>-new patch replacing 
>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.org%2Flkml%2F2017%2F7%2F17%2F419=01%7C01%7Claurentiu.tudor%40nxp.com%7C77381272b4914c9ac64708d4cde7d94e%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0=FTVLKox6T4i9OFmb%2B5BkSEDQrDrafXznY6nsJ0dgFSk%3D=0
>>
>>   drivers/staging/fsl-mc/bus/mc-sys.c | 21 +++--
>>   1 file changed, 15 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/staging/fsl-mc/bus/mc-sys.c 
>> b/drivers/staging/fsl-mc/bus/mc-sys.c
>> index 195d9f3..8a6dc47 100644
>> --- a/drivers/staging/fsl-mc/bus/mc-sys.c
>> +++ b/drivers/staging/fsl-mc/bus/mc-sys.c
>> @@ -126,12 +126,15 @@ static inline void mc_write_command(struct mc_command 
>> __iomem *portal,
>>
>>  /* copy command parameters into the portal */
>>  for (i = 0; i < MC_CMD_NUM_OF_PARAMS; i++)
>> -   __raw_writeq(cmd->params[i], >params[i]);
>> -   /* ensure command params are committed before submitting it */
>> -   wmb();
>> +   /*
>> +* Data is already in the expected LE byte-order. Do an
>> +* extra LE -> CPU conversion so that the CPU -> LE done in
>> +* the device io write api puts it back in the right order.
>> +*/
>> +   writeq_relaxed(le64_to_cpu(cmd->params[i]), 
>> >params[i]);
>>
>>  /* submit the command by writing the header */
>> -   __raw_writeq(cmd->header, >header);
>> +   writeq(le64_to_cpu(cmd->header), >header);
>>   }
>
> Looks good, but just to be sure this is what you intended:
>
> On 32-bit systems, this will now write val>>32 to cmd->header+4,
> followed by writing val&0x to cmd->header.

Right. That's how it should happen.

> You said earlier that the command is triggered when the final
> four bytes are written, but it looks like the order is wrong now.
>
> Should you use io-64-nonatomic-lo-hi.h instead of
> io-64-nonatomic-hi-lo.h then?
>

Maybe i made an error in my previous emails, but the hi-lo variant is
the correct one. The command execution is triggered when the _first_ 
32-bit half of the header (header&0x) is written, so that's why 
it must be written last.

---
Thanks & Best Regards, Laurentiu

Re: [PATCH 6/7] staging: fsl-mc: rewrite mc command send/receive to work on 32-bits

[Laurentiu Tudor]

On 07/17/2017 06:00 PM, Arnd Bergmann wrote:
> On Mon, Jul 17, 2017 at 4:27 PM, Laurentiu Tudor
> <laurentiu.tu...@nxp.com> wrote:
>> Hi Arnd,
>>
>> On 07/17/2017 04:45 PM, Arnd Bergmann wrote:
>>> On Mon, Jul 17, 2017 at 3:26 PM,  <laurentiu.tu...@nxp.com> wrote:
>>>> From: Laurentiu Tudor <laurentiu.tu...@nxp.com>
>>>>
>>>> Split the 64-bit accesses in 32-bit accesses because there's no real
>>>> constrain in MC to do only atomic 64-bit. There's only one place
>>>> where ordering is important: when writing the MC command header the
>>>> first 32-bit part of the header must be written last.
>>>> We do this switch in order to allow compiling the driver on 32-bit.
>>>>
>>>> Signed-off-by: Laurentiu Tudor <laurentiu.tu...@nxp.com>
>>>> ---
>>>>drivers/staging/fsl-mc/bus/mc-sys.c | 31 ---
>>>>1 file changed, 12 insertions(+), 19 deletions(-)
>>>>
>>>> diff --git a/drivers/staging/fsl-mc/bus/mc-sys.c 
>>>> b/drivers/staging/fsl-mc/bus/mc-sys.c
>>>> index 195d9f3..dd2828e 100644
>>>> --- a/drivers/staging/fsl-mc/bus/mc-sys.c
>>>> +++ b/drivers/staging/fsl-mc/bus/mc-sys.c
>>>> @@ -124,14 +124,15 @@ static inline void mc_write_command(struct 
>>>> mc_command __iomem *portal,
>>>>{
>>>>   int i;
>>>>
>>>> -   /* copy command parameters into the portal */
>>>> -   for (i = 0; i < MC_CMD_NUM_OF_PARAMS; i++)
>>>> -   __raw_writeq(cmd->params[i], >params[i]);
>>>> -   /* ensure command params are committed before submitting it */
>>>> -   wmb();
>>>> -
>>>> -   /* submit the command by writing the header */
>>>> -   __raw_writeq(cmd->header, >header);
>>>> +   /*
>>>> +* copy command parameters into the portal. Final write
>>>> +* triggers the submission of the command.
>>>> +*/
>>>> +   for (i = sizeof(struct mc_command) / sizeof(u32) - 1; i >= 0; i--) 
>>>> {
>>>> +   __raw_writel(((u32 *)cmd)[i], &((u32 *)portal)[i]);
>>>> +   /* ensure command params are committed before submitting 
>>>> it */
>>>> +   wmb();
>>>> +   }
>>>>}
>>>
>>> What is the byte order requirement on this buffer?
>>
>> Endianness is handled by the callers so this function must leave
>> the binary blob intact.
>
> Got it, the endianess looks correct indeed.
>
>>> If this is a byte string
>>> rather than individual registers, you should probably just use
>>> memcpy_toio()
>>
>> It's a header followed by an opaque command. The protocol for queueing a
>> command says that the first 32-bit half of the header must be written
>> last, this triggering the command handling in the MC.
>
> Strictly speaking the __raw_writel() won't guarantee that the
> data is written as a single word, the compiler might decide to
> split it up into byte-sized writes if it believes the destination pointer
> is unaligned and the CPU has no efficient writes.
>
> I think this cannot happen on arm or powerpc, as we go through
> inline assembly for the store, but it's not completely portable.

Should i worry about portability? Slim changes that this driver
will ever run on anything else other than ARM & ARM64.
My current goal was just to make it compile on other arches.

> Before your patch, both the compiler and the CPU could also
> reorder the stores in theory (but normally won't), but the wmb()
> will prevent that now.

Ok, thanks for the info.

>>> but if these are separate registers, then using the
>>> __raw_* accessors is still wrong, at least on kernels that have a
>>> different byteorder from the hardware.
>>
>> As mentioned above, endianness is handled by the caller. This function
>> takes raw data and must leave it unchanged.
>>
>>> Also, are you sure that adding those six extra barriers has no
>>> performance impact?
>>
>> This is a slow interface used in slow paths, so i don't think those
>> extra barriers will have any performance impact.
>
> Ok.
>
> One other problem remains: most developers looking at this
> code like Robin and me will immediately think there might be
> an endianess bug here. As this is a slow path, how about
> using an explicit conversion using
>
>   writeq(le64_to_cpup(buffer), pointer);

Sure, sounds perfect. I'll do that in the next respin.

---
Thanks & Best Regards, Laurentiu

Re: [PATCH v2 8/8] staging: fsl-mc: allow the driver compile multi-arch

[Laurentiu Tudor]

Hi Arnd,

On 07/18/2017 05:25 PM, Arnd Bergmann wrote:
> On Tue, Jul 18, 2017 at 3:37 PM,   wrote:
>
>> --- a/drivers/staging/fsl-dpaa2/Kconfig
>> +++ b/drivers/staging/fsl-dpaa2/Kconfig
>> @@ -4,7 +4,7 @@
>>
>>   config FSL_DPAA2
>>  bool "Freescale DPAA2 devices"
>> -   depends on FSL_MC_BUS
>> +   depends on FSL_MC_BUS && ARCH_LAYERSCAPE
>>  ---help---
>>Build drivers for Freescale DataPath Acceleration
>>Architecture (DPAA2) family of SoCs.
>
> I would probably leave the dependency in there conditionally, like
>
>   depends on ARCH_LAYERSCAPE || COMPILE_TEST
>
> That way, we can build the driver on all architectures with "make 
> allmodconfig"
> or "make randconfig", but regular users that disable COMPILE_TEST
> won't be bothered by the extra config options unless they have the
> right hardware.
>

Good point, I'll take care of it. But don't you mean COMPILE_TEST be 
added on the actual MC_BUS config, like so:

  config FSL_MC_BUS
bool "QorIQ DPAA2 fsl-mc bus driver"
-   depends on OF && ARCH_LAYERSCAPE
+   depends on OF && (ARCH_LAYERSCAPE || COMPILE_TEST)
select GENERIC_MSI_IRQ_DOMAIN
?

The other drivers that depend on the MC_BUS won't compile on other 
architectures.

---
Thanks & Best Regards, Laurentiu

Re: [PATCH v2 0/7] staging: fsl-mc: make the driver compile on other architectures

[Laurentiu Tudor]

Hi Arnd,

On 07/18/2017 05:26 PM, Arnd Bergmann wrote:
> On Tue, Jul 18, 2017 at 3:37 PM,  <laurentiu.tu...@nxp.com> wrote:
>> From: Laurentiu Tudor <laurentiu.tu...@nxp.com>
>>
>> Apart from a small change (first patch) which adds a missing comment,
>> this series make the bus driver compile on other architectures, as per
>> GregKH comment [1].
>> Compiled tested on:
>>   - booke powerpc (corenet{32,64}_smp_defconfig) with this ppc patch [2]
>>   - x86 (i386_defconfig, x86_64_defconfig, needs CONFIG_OF)
>>   - arm64 (defconfig)
>>
>> [1] 
>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.spinics.net%2Flists%2Flinux-driver-devel%2Fmsg100585.html=01%7C01%7Claurentiu.tudor%40nxp.com%7C2564747d26964ed5b3f408d4cde8f442%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0=Dz8vvLzQESel2UTXrhQ3JZyNhx5VhUdj6%2BE6TDLnXmc%3D=0
>> [2] 
>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.ozlabs.org%2Fpatch%2F789474%2F=01%7C01%7Claurentiu.tudor%40nxp.com%7C2564747d26964ed5b3f408d4cde8f442%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0=%2FiGSaa4j60INeTWEbT926iEIAtya6tqIiIoQN1yFbmA%3D=0
>
> Looks good overall. With the two minor questions addressed
>
> Acked-by: Arnd Bergmann <a...@arndb.de>
>

Thanks for the ack. I'll mention it the next version.

---
Thanks & Best Regards, Laurentiu

Re: [PATCH 7/7] staging: fsl-mc: allow the driver compile multi-arch

[Laurentiu Tudor]

Hi,

Sparc seems to be broken in multiple places, including generic code.
Is this a known issue?
Is there something I could/should do?

---
Thanks & Best Regards, Laurentiu

On 07/19/2017 08:31 PM, kbuild test robot wrote:
> Hi Laurentiu,
>
> [auto build test ERROR on staging/staging-testing]
> [also build test ERROR on v4.13-rc1 next-20170718]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
>
> url:
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2F0day-ci%2Flinux%2Fcommits%2Flaurentiu-tudor-nxp-com%2Fstaging-fsl-mc-make-the-driver-compile-on-other-architectures%2F20170718-021715=01%7C01%7Claurentiu.tudor%40nxp.com%7C66f7c5619d664f88ce6108d4cecc14f5%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0=HgKrckHRkhwe6PrGHc%2B1UfoT1rCyMw5C5%2B7wdEuvS5s%3D=0
> config: sparc-allyesconfig (attached as .config)
> compiler: sparc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
> reproduce:
>  wget 
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fraw.githubusercontent.com%2F01org%2Flkp-tests%2Fmaster%2Fsbin%2Fmake.cross=01%7C01%7Claurentiu.tudor%40nxp.com%7C66f7c5619d664f88ce6108d4cecc14f5%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0=kFjtWysm4LCK%2BnLcmpogI%2FHRRApSDRcl7QWofLo0%2FDY%3D=0
>  -O ~/bin/make.cross
>  chmod +x ~/bin/make.cross
>  # save the attached .config to linux build tree
>  make.cross ARCH=sparc
>
> All error/warnings (new ones prefixed by >>):
>
> In file included from kernel/irq/chip.c:14:0:
>>> include/linux/msi.h:225:10: error: unknown type name 'msi_alloc_info_t'
>   msi_alloc_info_t *arg);
>   ^~~~
> include/linux/msi.h:229:9: error: unknown type name 'msi_alloc_info_t'
>  msi_alloc_info_t *arg);
>  ^~~~
> include/linux/msi.h:238:12: error: unknown type name 'msi_alloc_info_t'
> msi_alloc_info_t *arg);
> ^~~~
> include/linux/msi.h:239:22: error: unknown type name 'msi_alloc_info_t'
>   void  (*msi_finish)(msi_alloc_info_t *arg, int retval);
>   ^~~~
> include/linux/msi.h:240:20: error: unknown type name 'msi_alloc_info_t'
>   void  (*set_desc)(msi_alloc_info_t *arg,
> ^~~~
> include/linux/msi.h:308:18: error: unknown type name 'msi_alloc_info_t'
> int nvec, msi_alloc_info_t *args);
>   ^~~~
> include/linux/msi.h:310:29: error: unknown type name 'msi_alloc_info_t'
>  int virq, int nvec, msi_alloc_info_t *args);
>  ^~~~
> --
> In file included from kernel/irq/msi.c:16:0:
>>> include/linux/msi.h:225:10: error: unknown type name 'msi_alloc_info_t'
>   msi_alloc_info_t *arg);
>   ^~~~
> include/linux/msi.h:229:9: error: unknown type name 'msi_alloc_info_t'
>  msi_alloc_info_t *arg);
>  ^~~~
> include/linux/msi.h:238:12: error: unknown type name 'msi_alloc_info_t'
> msi_alloc_info_t *arg);
> ^~~~
> include/linux/msi.h:239:22: error: unknown type name 'msi_alloc_info_t'
>   void  (*msi_finish)(msi_alloc_info_t *arg, int retval);
>   ^~~~
> include/linux/msi.h:240:20: error: unknown type name 'msi_alloc_info_t'
>   void  (*set_desc)(msi_alloc_info_t *arg,
> ^~~~
> include/linux/msi.h:308:18: error: unknown type name 'msi_alloc_info_t'
> int nvec, msi_alloc_info_t *args);
>   ^~~~
> include/linux/msi.h:310:29: error: unknown type name 'msi_alloc_info_t'
>  int virq, int nvec, msi_alloc_info_t *args);
>  ^~~~
> kernel/irq/msi.c: In function 'msi_domain_alloc':
>>> kernel/irq/msi.c:126:29: error: 'struct msi_domain_ops' has no member named 
>>> 'get_hwirq'
>   irq_hw_number_t hwirq = ops->get_hwirq(info, arg);
>  ^~
>>> kernel/irq/msi.c:139:12: error: 'struct msi_domain_ops' has no member named 
>>> 'msi_init'; did you mean 'msi_free'?
>ret = ops->msi_init(domain, info, virq + i, hwirq + i, arg);
> ^~
> kernel/irq/msi.c: At top level:
>>> kernel/irq/msi.c:201:11: error: unknown type name 'msi_alloc_info_t'
>msi_alloc_info_t *arg)
>^~~~
>>> kernel/irq/msi.c:221:2: error: unknown field 'get_hwirq' specified in 
>>> initializer
>   .get_hwirq = msi_domain_ops_get_hwirq,
>   ^
>>> kernel/irq/msi.c:222:2: error: unknown field 'msi_init' specified in 
>>> initializer
>   .msi_init = msi_domain_ops_init,
>   ^
>>> kernel/irq/msi.c:222:14: error: 'msi_domain_ops_init' undeclared here (not 
>>> in a function)
>   .msi_init 

Re: [PATCH] staging: fsl-mc: move bus driver out of staging

[Laurentiu Tudor]

On 08/23/2017 04:39 AM, Greg KH wrote:
> On Sat, Aug 19, 2017 at 08:18:12PM +0300, laurentiu.tu...@nxp.com wrote:
>> From: Stuart Yoder <stuart.yo...@nxp.com>
>>
>> Move the source files out of staging into their final locations:
>>-include files in drivers/staging/fsl-mc/include go to include/linux/fsl
>>-irq-gic-v3-its-fsl-mc-msi.c goes to drivers/irqchip
>>-source in drivers/staging/fsl-mc/bus goes to drivers/bus/fsl-mc
>>-README.txt, providing and overview of DPAA goes to
>> Documentation/dpaa2/overview.txt
>>
>> Update or delete other remaining staging files-- Makefile, Kconfig, TODO.
>> Update dpaa2_eth and dpio staging drivers.
>>
>> Signed-off-by: Stuart Yoder <stuyo...@gmail.com>
>> Signed-off-by: Laurentiu Tudor <laurentiu.tu...@nxp.com>
>> [Laurentiu: rebased, add dpaa2_eth and dpio #include updates]
>> Cc: Thomas Gleixner <t...@linutronix.de>
>> Cc: Jason Cooper <ja...@lakedaemon.net>
>> Cc: Marc Zyngier <marc.zyng...@arm.com>
>
> Really?  You all have fixed all of the previous issues found?

Well, it went through quite a few rounds of fixes and cleanups (driver 
model related + other fixes, cleanup [1], more cleanup, fixes [2], 
header file cleanup [3], compile multi-arch [4]) addressing the found 
issues.

> Is it time for another real review?  If so, please ask for it.

Yes, please review. Thanks in advance!

[1] https://lkml.org/lkml/2017/2/1/235
[2] https://www.spinics.net/lists/arm-kernel/msg586688.html
[3] https://www.spinics.net/lists/arm-kernel/msg590928.html
[4] https://lkml.org/lkml/2017/7/19/639
---
Thanks & Best Regards, Laurentiu

Re: [PATCH] powerpc: booke: fix boot crash due to null hugepd

[Laurentiu Tudor]

Hi Greg,

On 05/17/2017 12:15 PM, Greg KH wrote:
> On Tue, May 16, 2017 at 09:47:52AM -0500, laurentiu.tu...@nxp.com wrote:
>> From: Laurentiu Tudor <laurentiu.tu...@nxp.com>
>>
>> On 32-bit book-e machines, hugepd_ok() does not take
>> into account null hugepd values, causing this crash at boot:
>>
>
> $ ./scripts/get_maintainer.pl --file arch/powerpc/include/asm/nohash/pgtable.h
> Benjamin Herrenschmidt <b...@kernel.crashing.org> (supporter:LINUX FOR 
> POWERPC (32-BIT AND 64-BIT))
> Paul Mackerras <pau...@samba.org> (supporter:LINUX FOR POWERPC (32-BIT AND 
> 64-BIT))
> Michael Ellerman <m...@ellerman.id.au> (supporter:LINUX FOR POWERPC (32-BIT 
> AND 64-BIT),commit_signer:2/3=67%)
> "Aneesh Kumar K.V" <aneesh.ku...@linux.vnet.ibm.com> 
> (commit_signer:2/3=67%,authored:1/3=33%,added_lines:3/8=38%,removed_lines:2/3=67%)
> Scott Wood <o...@buserror.net> (commit_signer:1/3=33%)
> Laurentiu Tudor <laurentiu.tu...@nxp.com> 
> (commit_signer:1/3=33%,authored:1/3=33%,added_lines:1/8=12%,removed_lines:1/3=33%)
> Christophe Leroy <christophe.le...@c-s.fr> 
> (commit_signer:1/3=33%,authored:1/3=33%,added_lines:4/8=50%)
> linuxppc-...@lists.ozlabs.org (open list:LINUX FOR POWERPC (32-BIT AND 
> 64-BIT))
> linux-kernel@vger.kernel.org (open list)
>
> I'm not listed there at all, any specific reason you sent this to me?
> Nothing I can do with it...
>

Just some finger trouble on my side. Please disregard and sorry for the 
noise.

---
Best Regards, Laurentiu

Re: [PATCH 01/14] staging: fsl-mc: drop macros with possible side effects

[Laurentiu Tudor]

Hi Joe,

On 06/22/2017 07:07 PM, Joe Perches wrote:
> On Thu, 2017-06-22 at 16:35 +0300, laurentiu.tu...@nxp.com wrote:
>> From: Laurentiu Tudor <laurentiu.tu...@nxp.com>
>>
>> Several macros were triggering this checkpatch.pl warning:
>>"Macro argument reuse '$arg' - possible side-effects?"
>> Fix the warning by turning them into real functions.
>
> good idea and
>
>> diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c 
>> b/drivers/staging/fsl-mc/bus/dprc-driver.c
> []
>> +static bool fsl_mc_device_match(struct fsl_mc_device *mc_dev,
>> +struct dprc_obj_desc *obj_desc)
>> +{
>> +return !strcmp(mc_dev->obj_desc.type, obj_desc->type) &&
>> +mc_dev->obj_desc.id == obj_desc->id;
>> +}
>
> I'd reverse the test order and do the strcmp after the comparison
>
>   return mc_dev->obj_desc.id == obj_desc->id &&
>  !strcmp(mc_dev->obj_desc.type, obj_desc->type);
>
> []
>
>> +static bool __must_check fsl_mc_is_allocatable(const char *obj_type)
>> +{
>> +return strcmp(obj_type, "dpbp") == 0 ||
>> +   strcmp(obj_type, "dpmcp") == 0 ||
>> +   strcmp(obj_type, "dpcon") == 0;
>> +}
>
> please be consistent in using either == 0 or !
> when using strcmp
>

Thanks for the suggestions. Will take care of them in the next round.

---
Best Regards, Laurentiu

Re: [PATCH 1/2] staging: fsl-dpaa2/eth: Fix address translations

[Laurentiu Tudor]

Hi Ioana,

Debatable nit inline.

On 05/24/2017 03:13 PM, Ioana Radulescu wrote:
> Use the correct mechanisms for translating a DMA-mapped IOVA
> address into a virtual one. Without this fix, once SMMU is
> enabled on Layerscape platforms, the Ethernet driver throws
> IOMMU translation faults.
>
> Signed-off-by: Nipun Gupta 
> Signed-off-by: Ioana Radulescu 
> ---
>   drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 25 
> +++--
>   drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h |  1 +
>   2 files changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c 
> b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
> index 6f9eed66c64d..3fee0d6f17e0 100644
> --- a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
> +++ b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
> @@ -37,6 +37,7 @@
>   #include 
>   #include 
>   #include 
> +#include 
>
>   #include "../../fsl-mc/include/mc.h"
>   #include "../../fsl-mc/include/mc-sys.h"
> @@ -54,6 +55,16 @@ MODULE_DESCRIPTION("Freescale DPAA2 Ethernet Driver");
>
>   const char dpaa2_eth_drv_version[] = "0.1";
>
> +static void *dpaa2_iova_to_virt(struct iommu_domain *domain,

if you pass a "struct dpaa2_eth_priv *priv" instead of "iommu_domain" 
you can move the priv->iommu_domain reference in the function and 
slightly simplify the call sites.

---
Best Regards, Laurentiu

Re: [PATCH 1/5][v2] staging: fsl-mc: fix several checkpath.pl warnings

[Laurentiu Tudor]

Hi Greg,

On 05/25/2017 07:58 PM, Greg KH wrote:
> On Mon, May 22, 2017 at 03:09:31PM +0300, laurentiu.tu...@nxp.com wrote:
>> From: Laurentiu Tudor <laurentiu.tu...@nxp.com>
>
> Your subject line is very odd, please use the 'v2' marking properly...
>
>>
>> Remove several unneeded #includes, forward
>> declarations and fix several issues reported
>> by checkpatch.pl --strict, such as:
>>   - kfree(NULL) is safe and check is not required
>>   - macro argument reuse may cause possible side effects
>>   - enclose macro params in parens to avoid precedence issues
>>   - coding style
>
> These, as always, need to be broken up into one-patch-per-type-of-thing,
> you have been in the staging tree long enough to know this :(

Sorry about that. Will take care of all your comments in the next respin.

---
Thanks & Best Regards, Laurentiu

RE: [PATCH 3/3][v4] staging: fsl-mc: move bus driver out of staging

[Laurentiu Tudor]

Hi Marc,

> -Original Message-
> From: Marc Zyngier [mailto:marc.zyng...@arm.com]
> Sent: Monday, May 22, 2017 10:41 AM
> 
> On Mon, May 22 2017 at  7:12:39 am GMT, Laurentiu Tudor
> <laurentiu.tu...@nxp.com> wrote:
> 
> Hi Laurentiu,
> 
> > Hi Marc,
> >
> >> -Original Message-
> >> From: Marc Zyngier [mailto:marc.zyng...@arm.com]
> >> Sent: Saturday, May 20, 2017 9:43 AM
> >> To: Matthias Brugger <matthias@gmail.com>
> >>
> >> On Fri, May 19 2017 at 02:41:43 PM, Matthias Brugger
> >> <matthias@gmail.com> wrote:
> >> > On 19/05/17 15:13, laurentiu.tu...@nxp.com wrote:
> >> >> From: Stuart Yoder <stuart.yo...@nxp.com>
> >> >>
> >> >> Move the source files out of staging into their final locations:
> >> >>-include files in drivers/staging/fsl-mc/include go to 
> >> >> include/linux/fsl
> >> >>-irq-gic-v3-its-fsl-mc-msi.c goes to drivers/irqchip
> >> >
> >> > This driver has as compatible "arm,gic-v3-its". I wonder if this is
> >> > correct and if it should be moved like this out of staging.
> >>
> >> This is no different from the way we handle *any* bus that uses the
> >> GICv3 ITS as an MSI controller. Each bus provides its glue code that
> >> latches onto the ITS node, and calls into the generic code.
> >>
> >> Now, when it comes to moving this out of staging, here is my concern:
> >> There is mention of a userspace tool (restool) used to control the
> >> HW. Where is this tool? Where is the user ABI documented?
> >
> > The tool is published here:
> >
> > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit
> > hub.com%2Fqoriq-open-
> source%2Frestool=01%7C01%7Claurentiu.tudor%4
> >
> 0nxp.com%7Cd3c05908969d499cd4a008d4a0e5eaae%7C686ea1d3bc2b4c6fa92
> cd99c
> >
> 5c301635%7C0=2sEXCZ%2BAFlTtle8N3yWJPsGRve8cXMRPzyumlwqOhbg
> %3D
> > served=0
> >
> > There are two ways of configuring the mc-bus:
> >  - a static one, through a FDT based configuration file (we call it
> > DPL), documented in the refman linked below, chapter 22.
> >  - a dynamic one, using this restool utility.
> > Please note the usage of restool is optional.
> 
> Optional or not, it still is a userspace ABI, and while I can see restool 
> issuing ioctl
> system calls to configure the HW, I cannot see the corresponding code in the
> kernel tree. So how does it work?
> If the syscall interface is not present in the mainline kernel, drop the 
> reference
> to it in the documentation. If it is there (and I obviously missed it), 
> document it,
> and get it reviewed. 

Our original plan was to first get the bus out of staging and after that submit 
the restool support ASAP (patches are done - so I'm thinking at few days 
timeframe).
if this is not acceptable, I can drop the restool reference from the README and 
resubmit the patch series. We'll re-add the reference together with the restool 
support patches.

> If there are associated DT bindings to the kernel code, they
> must be documented (and reviewed) as part of the device-tree documentation,
> and not in some obscure, hard to access document.

There's only one binding involved and it's already accepted [1].

[snip]

> >
> > We're also working on publishing the docs on github so that they're
> > more accessible.
> 
> That'd be great, because the way the registration process is presented, I'd 
> have
> to agree to the Access Agreement *before* having a chance to read it. Not
> going to happen...

Sorry about that. Not much I can do. :-( 

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt

---
Best Regards, Laurentiu

RE: [PATCH 3/3][v4] staging: fsl-mc: move bus driver out of staging

[Laurentiu Tudor]

Hi Marc,

> -Original Message-
> From: Marc Zyngier [mailto:marc.zyng...@arm.com]
> Sent: Saturday, May 20, 2017 9:43 AM
> To: Matthias Brugger <matthias@gmail.com>
> Cc: Laurentiu Tudor <laurentiu.tu...@nxp.com>; gre...@linuxfoundation.org;
> stuyo...@gmail.com; de...@driverdev.osuosl.org; a...@arndb.de; Ruxandra
> Ioana Radulescu <ruxandra.radule...@nxp.com>; Stuart Yoder
> <stuart.yo...@nxp.com>; Roy Pledge <roy.ple...@nxp.com>; linux-
> ker...@vger.kernel.org; ag...@suse.de; Catalin Horghidan
> <catalin.horghi...@nxp.com>; Ioana Ciornei <ioana.cior...@nxp.com>;
> Thomas Gleixner <t...@linutronix.de>; Leo Li <leoyang...@nxp.com>; Bharat
> Bhushan <bharat.bhus...@nxp.com>; Jason Cooper <ja...@lakedaemon.net>;
> linux-arm-ker...@lists.infradead.org; Rob Herring <robh...@kernel.org>
> Subject: Re: [PATCH 3/3][v4] staging: fsl-mc: move bus driver out of staging
> Importance: High
> 
> On Fri, May 19 2017 at 02:41:43 PM, Matthias Brugger
> <matthias@gmail.com> wrote:
> > On 19/05/17 15:13, laurentiu.tu...@nxp.com wrote:
> >> From: Stuart Yoder <stuart.yo...@nxp.com>
> >>
> >> Move the source files out of staging into their final locations:
> >>-include files in drivers/staging/fsl-mc/include go to include/linux/fsl
> >>-irq-gic-v3-its-fsl-mc-msi.c goes to drivers/irqchip
> >
> > This driver has as compatible "arm,gic-v3-its". I wonder if this is
> > correct and if it should be moved like this out of staging.
> 
> This is no different from the way we handle *any* bus that uses the
> GICv3 ITS as an MSI controller. Each bus provides its glue code that latches 
> onto
> the ITS node, and calls into the generic code.
> 
> Now, when it comes to moving this out of staging, here is my concern:
> There is mention of a userspace tool (restool) used to control the HW. Where 
> is
> this tool? Where is the user ABI documented?

The tool is published here:

https://github.com/qoriq-open-source/restool

There are two ways of configuring the mc-bus:
 - a static one, through a FDT based configuration file (we call it DPL), 
documented in the refman linked below, chapter 22.
 - a dynamic one, using this restool utility.
Please note the usage of restool is optional.

The reference manual documenting the ABI can be found here (registration 
required):

https://freescale.sdlproducts.com/LiveContent/content/en-US/QorIQ_SDK/GUID-53BEBDD8-1A5E-4DD0-8354-A9647AD35755

Click on the DPAA2 user manual link.

We're also working on publishing the docs on github so that they're more 
accessible.

---
Thanks & Best Regards, Laurentiu

RE: [PATCH 3/3][v4] staging: fsl-mc: move bus driver out of staging

[Laurentiu Tudor]

> -Original Message-
> From: Marc Zyngier [mailto:marc.zyng...@arm.com]
> Sent: Monday, May 22, 2017 12:06 PM
> To: Laurentiu Tudor <laurentiu.tu...@nxp.com>
> 
> On 22/05/17 09:42, Laurentiu Tudor wrote:
> > Hi Marc,
> >
> >> -Original Message-
> >> From: Marc Zyngier [mailto:marc.zyng...@arm.com]
> >> Sent: Monday, May 22, 2017 10:41 AM
> >>
> >> On Mon, May 22 2017 at  7:12:39 am GMT, Laurentiu Tudor
> >> <laurentiu.tu...@nxp.com> wrote:
> >>
> >> Hi Laurentiu,
> >>
> >>> Hi Marc,
> >>>
> >>>> -Original Message-
> >>>> From: Marc Zyngier [mailto:marc.zyng...@arm.com]
> >>>> Sent: Saturday, May 20, 2017 9:43 AM
> >>>> To: Matthias Brugger <matthias@gmail.com>
> >>>>
> >>>> On Fri, May 19 2017 at 02:41:43 PM, Matthias Brugger
> >>>> <matthias@gmail.com> wrote:
> >>>>> On 19/05/17 15:13, laurentiu.tu...@nxp.com wrote:
> >>>>>> From: Stuart Yoder <stuart.yo...@nxp.com>
> >>>>>>
> >>>>>> Move the source files out of staging into their final locations:
> >>>>>>-include files in drivers/staging/fsl-mc/include go to 
> >>>>>> include/linux/fsl
> >>>>>>-irq-gic-v3-its-fsl-mc-msi.c goes to drivers/irqchip
> >>>>>
> >>>>> This driver has as compatible "arm,gic-v3-its". I wonder if this
> >>>>> is correct and if it should be moved like this out of staging.
> >>>>
> >>>> This is no different from the way we handle *any* bus that uses the
> >>>> GICv3 ITS as an MSI controller. Each bus provides its glue code
> >>>> that latches onto the ITS node, and calls into the generic code.
> >>>>
> >>>> Now, when it comes to moving this out of staging, here is my concern:
> >>>> There is mention of a userspace tool (restool) used to control the
> >>>> HW. Where is this tool? Where is the user ABI documented?
> >>>
> >>> The tool is published here:

[snip]

> >>> There are two ways of configuring the mc-bus:
> >>>  - a static one, through a FDT based configuration file (we call it
> >>> DPL), documented in the refman linked below, chapter 22.
> >>>  - a dynamic one, using this restool utility.
> >>> Please note the usage of restool is optional.
> >>
> >> Optional or not, it still is a userspace ABI, and while I can see
> >> restool issuing ioctl system calls to configure the HW, I cannot see
> >> the corresponding code in the kernel tree. So how does it work?
> >> If the syscall interface is not present in the mainline kernel, drop
> >> the reference to it in the documentation. If it is there (and I
> >> obviously missed it), document it, and get it reviewed.
> >
> > Our original plan was to first get the bus out of staging and after that 
> > submit
> the restool support ASAP (patches are done - so I'm thinking at few days
> timeframe).
> > if this is not acceptable, I can drop the restool reference from the README
> and resubmit the patch series. We'll re-add the reference together with the
> restool support patches.
> 
> I think it would make a lot more sense to drop anything that is not 
> implemented
> by the current code. Once you have patches that implement this userspace
> interface, they can be reviewed together with the corresponding
> documentation.

Ok, sounds good. I'll respin the patch series. 

> >> If there are associated DT bindings to the kernel code, they must be
> >> documented (and reviewed) as part of the device-tree documentation,
> >> and not in some obscure, hard to access document.
> >
> > There's only one binding involved and it's already accepted [1].
> 
> Ah, I missed it. It would be good to mention it in the documentation as well.

Good point. I'll add a reference in the next respin.

> > [snip]
> >
> >>>
> >>> We're also working on publishing the docs on github so that they're
> >>> more accessible.
> >>
> >> That'd be great, because the way the registration process is
> >> presented, I'd have to agree to the Access Agreement *before* having
> >> a chance to read it. Not going to happen...
> >
> > Sorry about that. Not much I can do. :-(
> 
> I understand this is not your job ;-). But maybe making people inside NXP 
> aware
> of the issue would help... Oh well.

I'll make sure your comments will reach our guys and in the meantime push to 
get the docs on github.

---
Thanks & Best Regards, Laurentiu

Re: [PATCH 1/2] staging: fsl-dpaa2/eth: Fix address translations

[Laurentiu Tudor]

On 05/25/2017 03:31 PM, Ruxandra Ioana Radulescu wrote:
>> -Original Message-
>> From: Laurentiu Tudor
>> Sent: Wednesday, May 24, 2017 3:34 PM
>> To: Ruxandra Ioana Radulescu <ruxandra.radule...@nxp.com>;
>> gre...@linuxfoundation.org
>> Cc: de...@driverdev.osuosl.org; linux-kernel@vger.kernel.org;
>> ag...@suse.de; a...@arndb.de; linux-arm-ker...@lists.infradead.org;
>> io...@lists.linux-foundation.org; Bogdan Purcareata
>> <bogdan.purcare...@nxp.com>; stuyo...@gmail.com; Nipun Gupta
>> <nipun.gu...@nxp.com>
>> Subject: Re: [PATCH 1/2] staging: fsl-dpaa2/eth: Fix address translations
>>
>> Hi Ioana,
>>
>> Debatable nit inline.
>>
>> On 05/24/2017 03:13 PM, Ioana Radulescu wrote:
>>> Use the correct mechanisms for translating a DMA-mapped IOVA
>>> address into a virtual one. Without this fix, once SMMU is
>>> enabled on Layerscape platforms, the Ethernet driver throws
>>> IOMMU translation faults.
>>>
>>> Signed-off-by: Nipun Gupta <nipun.gu...@nxp.com>
>>> Signed-off-by: Ioana Radulescu <ruxandra.radule...@nxp.com>
>>> ---
>>>drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 25
>> +++--
>>>drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h |  1 +
>>>2 files changed, 20 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
>> b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
>>> index 6f9eed66c64d..3fee0d6f17e0 100644
>>> --- a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
>>> +++ b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
>>> @@ -37,6 +37,7 @@
>>>#include 
>>>#include 
>>>#include 
>>> +#include 
>>>
>>>#include "../../fsl-mc/include/mc.h"
>>>#include "../../fsl-mc/include/mc-sys.h"
>>> @@ -54,6 +55,16 @@ MODULE_DESCRIPTION("Freescale DPAA2 Ethernet
>> Driver");
>>>
>>>const char dpaa2_eth_drv_version[] = "0.1";
>>>
>>> +static void *dpaa2_iova_to_virt(struct iommu_domain *domain,
>>
>> if you pass a "struct dpaa2_eth_priv *priv" instead of "iommu_domain"
>> you can move the priv->iommu_domain reference in the function and
>> slightly simplify the call sites.
>
> Fair point, but I'd prefer keeping this function independent of the
> Ethernet driver's private data structure. This way, if other (future)
> DPAA2 drivers will need a similar function, we can just move it
> to a common area instead of duplicating the code.

Understood. Fine by me then.

---
Best Regards, Laurentiu

Re: [PATCH v4 10/10] staging: fsl-mc: move bus driver out of staging

[Laurentiu Tudor]

Hi Marc,

On 05/31/2017 02:15 PM, Marc Zyngier wrote:
> On 31/05/17 11:58, laurentiu.tu...@nxp.com wrote:
>> From: Stuart Yoder <stuart.yo...@nxp.com>
>>
>> Move the source files out of staging into their final locations:
>>-include files in drivers/staging/fsl-mc/include go to include/linux/fsl
>>-irq-gic-v3-its-fsl-mc-msi.c goes to drivers/irqchip
>>-source in drivers/staging/fsl-mc/bus goes to drivers/bus/fsl-mc
>>-README.txt, providing and overview of DPAA goes to
>> Documentation/dpaa2/overview.txt
>>
>> Update or delete other remaining staging files-- Makefile, Kconfig, TODO.
>> Update dpaa2_eth and dpio staging drivers.
>>
>> Signed-off-by: Stuart Yoder <stuyo...@gmail.com>
>> Signed-off-by: Laurentiu Tudor <laurentiu.tu...@nxp.com>
>> [Laurentiu: rebased, add dpaa2_eth and dpio #include updates]
>> Cc: Thomas Gleixner <t...@linutronix.de>
>> Cc: Jason Cooper <ja...@lakedaemon.net>
>> Cc: Marc Zyngier <marc.zyng...@arm.com>
>> ---
>>
>> Notes:
>>  -no changes since v4
>>  -v4
>>-rebased
>>-update existing dpaa2 drivers to work with the bus out of staging
>>  -v3
>>-no changes
>>  -v2
>>-updated MAINTAINERS with new location
>>
>>   .../README.txt => Documentation/dpaa2/overview.txt|  0
>
> I thought you had agreed to drop all references to the userspace
> interface (restool and co) which is completely absent from this code?
> I'm certainly not going to ack this until this is done.

That's in patch 08/10 [1] but i guess you didn't get to see it because, 
as you mention, i forgot to Cc you on the whole series.
My bad, sorry about it. :-(

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

---
Best Regards, Laurentiu

RE: [PATCH v4 10/10] staging: fsl-mc: move bus driver out of staging

[Laurentiu Tudor]

> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Tuesday, June 06, 2017 5:20 PM
> To: Laurentiu Tudor <laurentiu.tu...@nxp.com>
> 
> On Tue, Jun 06, 2017 at 02:03:44PM +, Laurentiu Tudor wrote:
> > Hi Greg,
> >
> > > -Original Message-
> > > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > > Sent: Saturday, June 03, 2017 11:40 AM
> > >
> > > On Wed, May 31, 2017 at 01:58:52PM +0300, laurentiu.tu...@nxp.com
> wrote:
> > > > From: Stuart Yoder <stuart.yo...@nxp.com>
> > >
> > > Your subject says 'v4' yet the other patches here in the series do
> > > not, they say nothing, or 'v2', and the 0/10 patch says 'v6'!
> >
> > I'm tracking the patch versions individually and use the cover letter
> > version (v6 currently) to reflect the version of the patch series as a
> > whole. There are a bunch of patches that are either newly introduced in this
> series or didn't had multiple versions so I didn't tagged them with a version 
> tag.
> > The patch [10/10] didn't changed since v4 so I left the version
> > unchanged. The only change was in patch [07/10] for which I added a commit
> message so I bumped the version to v2.
> 
> And that's totally crazy.  What do you expect me, or anyone else, to know what
> to do with that.  Heck, I can't even sort them by proper order in which to 
> apply
> them in, if I wanted to.  You are keeping me from actually using your work :(
> 
> > > Please fix up and do this correctly.  The _whole_ patch series is
> > > versioned, otherwise how can I sort them to apply them in the correct 
> > > order?
> >
> > Want me to tag all the patches with the same version (that is, v7 for
> > the next respin)?
> 
> Yes, like everyone else does please :)
> 

Ok, finally got it. :-) Will do so in the next spin.

---
Thanks & Best Regards, Laurentiu

RE: [PATCH v4 10/10] staging: fsl-mc: move bus driver out of staging

[Laurentiu Tudor]

Hi Greg, 

> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Saturday, June 03, 2017 11:40 AM
> 
> On Wed, May 31, 2017 at 01:58:52PM +0300, laurentiu.tu...@nxp.com wrote:
> > From: Stuart Yoder 
> 
> Your subject says 'v4' yet the other patches here in the series do not, they 
> say
> nothing, or 'v2', and the 0/10 patch says 'v6'!

I'm tracking the patch versions individually and use the cover letter version 
(v6 currently)
to reflect the version of the patch series as a whole. There are a bunch of 
patches that are either newly
introduced in this series or didn't had multiple versions so I didn't tagged 
them with a version tag.
The patch [10/10] didn't changed since v4 so I left the version unchanged. The 
only change was in patch [07/10]
for which I added a commit message so I bumped the version to v2.
 
> Please fix up and do this correctly.  The _whole_ patch series is versioned,
> otherwise how can I sort them to apply them in the correct order?

Want me to tag all the patches with the same version (that is, v7 for the next 
respin)?

---
Thanks & Best Regards, Laurentiu

Re: [PATCH v7 02/10] staging: fsl-mc: fix macros with possible side effects

[Laurentiu Tudor]

On 06/13/2017 01:12 PM, Greg KH wrote:
> On Thu, Jun 08, 2017 at 05:28:47PM +0300, laurentiu.tu...@nxp.com wrote:
>> From: Laurentiu Tudor <laurentiu.tu...@nxp.com>
>>
>> Several macros were triggering this checkpatch.pl warning:
>>"Macro argument reuse '$arg' - possible side-effects?"
>> Fix the warning by avoiding multiple macro argument use.
>>
>> Signed-off-by: Laurentiu Tudor <laurentiu.tu...@nxp.com>
>> ---
>>
>> Notes:
>>  -v7
>>-no changes
>>
>>   drivers/staging/fsl-mc/bus/dprc-driver.c  | 10 +++---
>>   drivers/staging/fsl-mc/bus/fsl-mc-allocator.c | 11 +++
>>   2 files changed, 14 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c 
>> b/drivers/staging/fsl-mc/bus/dprc-driver.c
>> index d723c69..39c9a3b 100644
>> --- a/drivers/staging/fsl-mc/bus/dprc-driver.c
>> +++ b/drivers/staging/fsl-mc/bus/dprc-driver.c
>> @@ -21,9 +21,13 @@
>>
>>   #define FSL_MC_DPRC_DRIVER_NAME"fsl_mc_dprc"
>>
>> -#define FSL_MC_DEVICE_MATCH(_mc_dev, _obj_desc) \
>> -(strcmp((_mc_dev)->obj_desc.type, (_obj_desc)->type) == 0 && \
>> - (_mc_dev)->obj_desc.id == (_obj_desc)->id)
>> +#define FSL_MC_DEVICE_MATCH(_mc_dev, _obj_desc) 
>> \
>> +({  \
>> +struct fsl_mc_device *__mc_dev = _mc_dev;   \
>> +struct dprc_obj_desc *__obj_desc = _obj_desc;   \
>> +(strcmp(__mc_dev->obj_desc.type, __obj_desc->type) == 0 &&  \
>> +__mc_dev->obj_desc.id == __obj_desc->id);   \
>> +})
>
> Ick, no.  Just make this a real function please.
>
>>
>>   struct dprc_child_objs {
>>  int child_count;
>> diff --git a/drivers/staging/fsl-mc/bus/fsl-mc-allocator.c 
>> b/drivers/staging/fsl-mc/bus/fsl-mc-allocator.c
>> index ce07096..d3def40 100644
>> --- a/drivers/staging/fsl-mc/bus/fsl-mc-allocator.c
>> +++ b/drivers/staging/fsl-mc/bus/fsl-mc-allocator.c
>> @@ -17,10 +17,13 @@
>>   #include "dpcon-cmd.h"
>>   #include "fsl-mc-private.h"
>>
>> -#define FSL_MC_IS_ALLOCATABLE(_obj_type) \
>> -(strcmp(_obj_type, "dpbp") == 0 || \
>> - strcmp(_obj_type, "dpmcp") == 0 || \
>> - strcmp(_obj_type, "dpcon") == 0)
>> +#define FSL_MC_IS_ALLOCATABLE(_obj_type)\
>> +({  \
>> +const char *__obj_type = _obj_type; \
>> +(strcmp(__obj_type, "dpbp") == 0 || \
>> + strcmp(__obj_type, "dpmcp") == 0 ||\
>> + strcmp(__obj_type, "dpcon") == 0); \
>> +})
>
> Same here.  Don't put real logic in a #define, it never makes sense to
> do it and only makes things much harder to debug.
>

Ok, will do. Lets drop this patch and i'll send another one changing all 
these to functions.

---
Thanks & Best Regards, Laurentiu

Re: [PATCH v7 10/10] staging: fsl-mc: move bus driver out of staging

[Laurentiu Tudor]

Hi Greg,

On 06/13/2017 01:22 PM, Greg KH wrote:
> On Thu, Jun 08, 2017 at 05:28:55PM +0300, laurentiu.tu...@nxp.com wrote:
>> From: Stuart Yoder 
>>
>> Move the source files out of staging into their final locations:
>>-include files in drivers/staging/fsl-mc/include go to include/linux/fsl
>>-irq-gic-v3-its-fsl-mc-msi.c goes to drivers/irqchip
>
> Why do you have so many different .h files?  You should only need 1
> "external" one, and one "internal" one, right?  Can you please work on
> cleaning that up first?
>

So here's the list of headers, for quick reference.

dpbp.h
dpcon-cmd.h
dpmng.h
dprc.h
mc-bus.h
mc-cmd.h
mc-sys.h
mc.h

And here's a proposal on how to reorganize them:

  - dpbp.h (together with dbbp.c) be left behind in staging as they are
not used by the bus itself but by the drivers probing on this bus.
They will be moved out of staging at a later time.
  - same for dpcon-cmd.h. Will handle it when we'll start work on
getting dpcon.c & dpcon.h out of staging.
  - dprc.h contains APIs for handling mc-bus "device containers" that are
managed by the mc-bus driver itself. I'd leave this as is, but i
think i can make it private.
  - regarding the multiple mc*.h files, i'll see what it takes to
refactor them in a mc-bus.h + mc-bus-private.h
  - dpmng.h merged in the public header

Regarding the future plans for dpbp.h and dpcon.h, these expose common 
APIs used throughout all the drivers, so i think it makes sense to leave 
them as they are and, when their time comes, move them in the public 
include/linux/fsl.

---
Thanks & Best Regards, Laurentiu

RE: [PATCH 01/10] staging: fsl-mc: enclose macro params in parens

[Laurentiu Tudor]

Hi Greg,

> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Saturday, June 03, 2017 11:41 AM
> 
> On Wed, May 31, 2017 at 01:58:43PM +0300, laurentiu.tu...@nxp.com wrote:
> > From: Laurentiu Tudor <laurentiu.tu...@nxp.com>
> >
> > Several macros didn't had macro params enclosed in parens. Fix them to
> > avoid precedence issues.
> >
> > Found with checkpatch.pl who was issuing this
> > message:
> >   "Macro argument 'id' may be better as '(id)'
> >to avoid precedence issues"
> 
> Why are you line-wrapping at such a small number?  Please use 72 columns like
> git asks you to...

I'll rewrap the commit message in the next spin.

---
Thanks & Best Regards, Laurentiu

RE: [PATCH v2] staging: fsl-mc: move bus driver out of staging

[Laurentiu Tudor]

Hi,

> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Thursday, August 31, 2017 7:05 PM
> 
> On Mon, Aug 28, 2017 at 01:54:05PM +0300, laurentiu.tu...@nxp.com wrote:
> > From: Stuart Yoder <stuart.yo...@nxp.com>
> >
> > Move the source files out of staging into their final locations:
> >   -include files in drivers/staging/fsl-mc/include go to include/linux/fsl
> >   -irq-gic-v3-its-fsl-mc-msi.c goes to drivers/irqchip
> >   -source in drivers/staging/fsl-mc/bus goes to drivers/bus/fsl-mc
> >   -README.txt, providing and overview of DPAA goes to
> >Documentation/dpaa2/overview.txt
> >
> > Update or delete other remaining staging files-- Makefile, Kconfig, TODO.
> > Update dpaa2_eth and dpio staging drivers.
> >
> > Signed-off-by: Stuart Yoder <stuyo...@gmail.com>
> > Signed-off-by: Laurentiu Tudor <laurentiu.tu...@nxp.com>
> > [Laurentiu: rebased, add dpaa2_eth and dpio #include updates]
> > Cc: Thomas Gleixner <t...@linutronix.de>
> > Cc: Jason Cooper <ja...@lakedaemon.net>
> > Cc: Marc Zyngier <marc.zyng...@arm.com>
> 
> This is going to have to wait until I get a chunk of time to do the review.
> Probably after 4.13-final is out.

Any updates on this? Any chances to make it in 4.14? Thanks!

---
Best Regards, Laurentiu

RE: [PATCH v2] staging: fsl-mc: move bus driver out of staging

[Laurentiu Tudor]

> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Monday, September 11, 2017 4:03 PM
> 
> On Mon, Sep 11, 2017 at 11:55:22AM +0000, Laurentiu Tudor wrote:
> > Hi,
> >
> > > -Original Message-
> > > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > > Sent: Thursday, August 31, 2017 7:05 PM
> > >
> > > On Mon, Aug 28, 2017 at 01:54:05PM +0300, laurentiu.tu...@nxp.com
> wrote:
> > > > From: Stuart Yoder <stuart.yo...@nxp.com>
> > > >
> > > > Move the source files out of staging into their final locations:
> > > >   -include files in drivers/staging/fsl-mc/include go to 
> > > > include/linux/fsl
> > > >   -irq-gic-v3-its-fsl-mc-msi.c goes to drivers/irqchip
> > > >   -source in drivers/staging/fsl-mc/bus goes to drivers/bus/fsl-mc
> > > >   -README.txt, providing and overview of DPAA goes to
> > > >Documentation/dpaa2/overview.txt
> > > >
> > > > Update or delete other remaining staging files-- Makefile, Kconfig, 
> > > > TODO.
> > > > Update dpaa2_eth and dpio staging drivers.
> > > >
> > > > Signed-off-by: Stuart Yoder <stuyo...@gmail.com>
> > > > Signed-off-by: Laurentiu Tudor <laurentiu.tu...@nxp.com>
> > > > [Laurentiu: rebased, add dpaa2_eth and dpio #include updates]
> > > > Cc: Thomas Gleixner <t...@linutronix.de>
> > > > Cc: Jason Cooper <ja...@lakedaemon.net>
> > > > Cc: Marc Zyngier <marc.zyng...@arm.com>
> > >
> > > This is going to have to wait until I get a chunk of time to do the 
> > > review.
> > > Probably after 4.13-final is out.
> >
> > Any updates on this? Any chances to make it in 4.14? Thanks!
> 
> No, it's too late for 4.14, it would have had to be in linux-next before 
> 4.13-final
> came out.  I'm at a conference all this week, 

Plumbers? :-)

> if I'm stuck in a boring talk, I'll try to do the review :)

Great. Thanks a lot for taking the time!

---
Best Regards, Laurentiu

Re: mpic IRQ_TYPE_BOTH handling

[Laurentiu Tudor]

On 08/31/2017 01:52 AM, Michael Ellerman wrote:
> Hi Gregory,
>
> Gregory Fong  writes:
>> Hi all,
>>
>> In arch/powerpc/sysdev/mpic.c , it looks like IRQ_TYPE_EDGE_BOTH is
>> handled the same way as IRQ_TYPE_EDGE_FALLING:
>>
>> static unsigned int mpic_type_to_vecpri(struct mpic *mpic, unsigned int type)
>> {
>>  /* Now convert sense value */
>>  switch(type & IRQ_TYPE_SENSE_MASK) {
>>  case IRQ_TYPE_EDGE_RISING:
>>  return MPIC_INFO(VECPRI_SENSE_EDGE) |
>> MPIC_INFO(VECPRI_POLARITY_POSITIVE);
>>  case IRQ_TYPE_EDGE_FALLING:
>>  case IRQ_TYPE_EDGE_BOTH:
>>  return MPIC_INFO(VECPRI_SENSE_EDGE) |
>> MPIC_INFO(VECPRI_POLARITY_NEGATIVE);
>>  case IRQ_TYPE_LEVEL_HIGH:
>>  return MPIC_INFO(VECPRI_SENSE_LEVEL) |
>> MPIC_INFO(VECPRI_POLARITY_POSITIVE);
>>  case IRQ_TYPE_LEVEL_LOW:
>>  default:
>>  return MPIC_INFO(VECPRI_SENSE_LEVEL) |
>> MPIC_INFO(VECPRI_POLARITY_NEGATIVE);
>>  }
>> }
>>
>> If IRQ_TYPE_EDGE_BOTH is unsupported, shouldn't we be returning an
>> error, instead of silently setting to use IRQ_TYPE_EDGE_FALLING?
>> Something like the following (sorry if the diff wraps weirdly, on
>> webmail at the moment):
>
> I don't know this code so I asked Ben and he said something like
> "PowerMacs never use BOTH, so it hasn't mattered, but Freescale machines
> might".

IIRC, the mpic in freescale MPICs the interrupts are either low or high, 
so not both. There's a bit which controls the interrupt polarity which 
selects if the interrupt triggers on high-to-low or low-to-high.
So i guess it doesn't matter on freescale machines too.

---
Best Regards, Laurentiu

Re: [RESEND PATCH 2/6] staging: fsl-dpaa2/ethsw: Add Freescale DPAA2 Ethernet Switch driver

[Laurentiu Tudor]

Hi Florian,

On 09/29/2017 07:11 PM, Florian Fainelli wrote:
> On September 29, 2017 6:59:18 AM PDT, Razvan Stefanescu 
> <razvan.stefane...@nxp.com> wrote:
>>
>>
>>> -Original Message-
>>> From: Bogdan Purcareata
>>> Sent: Friday, September 29, 2017 16:36
>>> To: Razvan Stefanescu <razvan.stefane...@nxp.com>;
>>> gre...@linuxfoundation.org
>>> Cc: de...@driverdev.osuosl.org; linux-kernel@vger.kernel.org;
>>> net...@vger.kernel.org; ag...@suse.de; a...@arndb.de; Alexandru
>> Marginean
>>> <alexandru.margin...@nxp.com>; Ruxandra Ioana Radulescu
>>> <ruxandra.radule...@nxp.com>; Laurentiu Tudor
>> <laurentiu.tu...@nxp.com>;
>>> stuyo...@gmail.com
>>> Subject: RE: [RESEND PATCH 2/6] staging: fsl-dpaa2/ethsw: Add
>> Freescale DPAA2
>>> Ethernet Switch driver
>>>
>>>> Introduce the DPAA2 Ethernet Switch driver, which manages Datapath
>> Switch
>>>> (DPSW) objects discovered on the MC bus.
>>>>
>>>> Suggested-by: Alexandru Marginean <alexandru.margin...@nxp.com>
>>>> Signed-off-by: Razvan Stefanescu <razvan.stefane...@nxp.com>
>
> This looks pretty good for a new switchdev driver, is there a reason you 
> can't target drivers/net/ethernet instead of staging? Is it because the MC 
> bus code is still in staging (AFAICT)?

There's a pending patch [1] that moves the bus from staging to its place 
in drivers/bus. DPIO plus other bits and pieces are next on the list.

Greg,

Just a heads up: if this driver goes in first, then i'll need to re-spin 
my patch [1] to also update the #include's in this switch driver.

P.S. Any news on my patch? :-)

[1] https://www.spinics.net/lists/arm-kernel/msg603534.html

---
Thanks, Laurentiu

Re: [PATCH v2] staging: fsl-mc: move bus driver out of staging

[Laurentiu Tudor]

On 11/03/2017 05:17 PM, Greg KH wrote:
> On Thu, Aug 31, 2017 at 06:04:30PM +0200, Greg KH wrote:
>> On Mon, Aug 28, 2017 at 01:54:05PM +0300, laurentiu.tu...@nxp.com wrote:
>>> From: Stuart Yoder <stuart.yo...@nxp.com>
>>>
>>> Move the source files out of staging into their final locations:
>>>-include files in drivers/staging/fsl-mc/include go to include/linux/fsl
>>>-irq-gic-v3-its-fsl-mc-msi.c goes to drivers/irqchip
>>>-source in drivers/staging/fsl-mc/bus goes to drivers/bus/fsl-mc
>>>-README.txt, providing and overview of DPAA goes to
>>> Documentation/dpaa2/overview.txt
>>>
>>> Update or delete other remaining staging files-- Makefile, Kconfig, TODO.
>>> Update dpaa2_eth and dpio staging drivers.
>>>
>>> Signed-off-by: Stuart Yoder <stuyo...@gmail.com>
>>> Signed-off-by: Laurentiu Tudor <laurentiu.tu...@nxp.com>
>>> [Laurentiu: rebased, add dpaa2_eth and dpio #include updates]
>>> Cc: Thomas Gleixner <t...@linutronix.de>
>>> Cc: Jason Cooper <ja...@lakedaemon.net>
>>> Cc: Marc Zyngier <marc.zyng...@arm.com>
>>
>> This is going to have to wait until I get a chunk of time to do the
>> review.  Probably after 4.13-final is out.
>
> Ok, review comments as I go through the code:
>   mc-sys.c 323 EXPORT_SYMBOL(mc_send_command);
>
> should be EXPORT_SYMBOL_GPL(fsl_mc_send_command); to match up with your
> other exports and global namespace, right?
>
> You export a lot of dpcon_* symbols that no one uses, please do not do
> that.  Verify that all of them are actually used right now, if not,
> remove them.  If you think you are going to use them in the future,
> wonderful, add them in then.
>
> Same for your dpaa2_* exported symbols, most are not used from what I
> can see.
>
> struct dpaa2_io {
>  atomic_t refs;
>
> That's a kref, please use it instead of trying to roll your own.
>
> And even for this, your locking is not correct (i.e. you do not have
> any), that needs to be fixed so that teardown works correctly.
>
> You have a lot of WARN_ON() calls, that's going to be ignored and should
> all not be needed now that the code is debugged and working properly.
> Please remove them, or turn them into dev_err() calls with a real if ()
> check instead.
>
> You are checking "strings" for the type of device in a lot of places,
> like this:
>   if (strcmp(obj_desc->type, "dprc") == 0) {
> why are you not just using the built-in driver model .type field and
> comparing that to the different type structures?  It's much easier, and
> you don't have to again, "roll your own".  See the USB or Greybus code
> for examples of busses that have different "types" of devices on them at
> the same time.
>
> Ok, that's enough for now, please work on this, and we can go from
> there...
>

What would the next steps be, now that the patches are in staging-next?
Are there plans for a new round of review?

---
Thanks & Best Regards, Laurentiu

Re: [PATCH v2] staging: fsl-mc: move bus driver out of staging

[Laurentiu Tudor]

On 11/28/2017 02:59 PM, Greg KH wrote:
> On Mon, Nov 27, 2017 at 03:32:28PM +0000, Laurentiu Tudor wrote:
>>
>>
>> On 11/03/2017 05:17 PM, Greg KH wrote:
>>> On Thu, Aug 31, 2017 at 06:04:30PM +0200, Greg KH wrote:
>>>> On Mon, Aug 28, 2017 at 01:54:05PM +0300, laurentiu.tu...@nxp.com wrote:
>>>>> From: Stuart Yoder <stuart.yo...@nxp.com>
>>>>>
>>>>> Move the source files out of staging into their final locations:
>>>>> -include files in drivers/staging/fsl-mc/include go to 
>>>>> include/linux/fsl
>>>>> -irq-gic-v3-its-fsl-mc-msi.c goes to drivers/irqchip
>>>>> -source in drivers/staging/fsl-mc/bus goes to drivers/bus/fsl-mc
>>>>> -README.txt, providing and overview of DPAA goes to
>>>>>  Documentation/dpaa2/overview.txt
>>>>>
>>>>> Update or delete other remaining staging files-- Makefile, Kconfig, TODO.
>>>>> Update dpaa2_eth and dpio staging drivers.
>>>>>
>>>>> Signed-off-by: Stuart Yoder <stuyo...@gmail.com>
>>>>> Signed-off-by: Laurentiu Tudor <laurentiu.tu...@nxp.com>
>>>>> [Laurentiu: rebased, add dpaa2_eth and dpio #include updates]
>>>>> Cc: Thomas Gleixner <t...@linutronix.de>
>>>>> Cc: Jason Cooper <ja...@lakedaemon.net>
>>>>> Cc: Marc Zyngier <marc.zyng...@arm.com>
>>>>
>>>> This is going to have to wait until I get a chunk of time to do the
>>>> review.  Probably after 4.13-final is out.
>>>
>>> Ok, review comments as I go through the code:
>>> mc-sys.c 323 EXPORT_SYMBOL(mc_send_command);
>>>
>>> should be EXPORT_SYMBOL_GPL(fsl_mc_send_command); to match up with your
>>> other exports and global namespace, right?
>>>
>>> You export a lot of dpcon_* symbols that no one uses, please do not do
>>> that.  Verify that all of them are actually used right now, if not,
>>> remove them.  If you think you are going to use them in the future,
>>> wonderful, add them in then.
>>>
>>> Same for your dpaa2_* exported symbols, most are not used from what I
>>> can see.
>>>
>>> struct dpaa2_io {
>>>   atomic_t refs;
>>>
>>> That's a kref, please use it instead of trying to roll your own.
>>>
>>> And even for this, your locking is not correct (i.e. you do not have
>>> any), that needs to be fixed so that teardown works correctly.
>>>
>>> You have a lot of WARN_ON() calls, that's going to be ignored and should
>>> all not be needed now that the code is debugged and working properly.
>>> Please remove them, or turn them into dev_err() calls with a real if ()
>>> check instead.
>>>
>>> You are checking "strings" for the type of device in a lot of places,
>>> like this:
>>> if (strcmp(obj_desc->type, "dprc") == 0) {
>>> why are you not just using the built-in driver model .type field and
>>> comparing that to the different type structures?  It's much easier, and
>>> you don't have to again, "roll your own".  See the USB or Greybus code
>>> for examples of busses that have different "types" of devices on them at
>>> the same time.
>>>
>>> Ok, that's enough for now, please work on this, and we can go from
>>> there...
>>>
>>
>> What would the next steps be, now that the patches are in staging-next?
>> Are there plans for a new round of review?
>
> Send a patch that moves the files you think should be moved at this
> point in time, as I'm not quite sure which ones exactly you feel are
> ready to go.

The initial plan was to strictly move the things needed by this bus 
driver, thus leaving some source files at a later time. To the point, 
these files:
   "drivers/staging/fsl-mc/bus/dpbp*.*"
   "drivers/staging/fsl-mc/bus/dpcon*.*"
and this whole dir:
   "drivers/staging/fsl-mc/bus/dpio/*".

I'll prepare the patch so that it's more visible what files are to be moved.

---
Thanks & Best Regards, Laurentiu

Re: [PATCH v3] staging: fsl-mc: use 32bits to support 64K size mc-portals

[Laurentiu Tudor]

On 11/22/2017 09:48 AM, Bharat Bhushan wrote:
> As per APIs each mc-portal is of 64K size while currently
> 16bits (type u16) is used to store size of mc-portal.
> In these cases upper bit of portal size gets truncated.
>
> Signed-off-by: Bharat Bhushan <bharat.bhus...@nxp.com>
> ---

Ok, so just to clarify this fixes the case where size is equal (or maybe 
larger in the future) to 0x10000.

Acked-By: Laurentiu Tudor <laurentiu.tu...@nxp.com>

---
Best Regards, Laurentiu

Re: [PATCH v3] staging: fsl-mc: move bus driver out of staging

[Laurentiu Tudor]

Hi Andrew,

On 11/28/2017 06:04 PM, Andrew Lunn wrote:
> On Tue, Nov 28, 2017 at 05:27:57PM +0200, laurentiu.tu...@nxp.com wrote:
>> diff --git a/drivers/staging/fsl-mc/bus/dpmcp.h b/drivers/bus/fsl-mc/dpmcp.h
>> similarity index 100%
>> rename from drivers/staging/fsl-mc/bus/dpmcp.h
>> rename to drivers/bus/fsl-mc/dpmcp.h
>> diff --git a/drivers/staging/fsl-mc/bus/dpmng-cmd.h 
>> b/drivers/bus/fsl-mc/dpmng-cmd.h
>> similarity index 100%
>> rename from drivers/staging/fsl-mc/bus/dpmng-cmd.h
>> rename to drivers/bus/fsl-mc/dpmng-cmd.h
>> diff --git a/drivers/staging/fsl-mc/bus/dprc-cmd.h 
>> b/drivers/bus/fsl-mc/dprc-cmd.h
>> similarity index 100%
>> rename from drivers/staging/fsl-mc/bus/dprc-cmd.h
>> rename to drivers/bus/fsl-mc/dprc-cmd.h
>> diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c 
>> b/drivers/bus/fsl-mc/dprc-driver.c
>> similarity index 99%
>> rename from drivers/staging/fsl-mc/bus/dprc-driver.c
>> rename to drivers/bus/fsl-mc/dprc-driver.c
>
> Hi Laurentiu
>
> It is hard to review code when we don't see it.

I'll reformat the patch with rename detection disabled, but please note 
that it will be pretty big (~370KiB).

---
Best Regards, Laurentiu

Re: [PATCH] staging: fsl-mc: fix uninitialized variable use

[Laurentiu Tudor]

On 11/27/2017 11:08 AM, Greg KH wrote:
> On Mon, Nov 27, 2017 at 11:01:34AM +0200, laurentiu.tu...@nxp.com wrote:
>> From: Laurentiu Tudor <laurentiu.tu...@nxp.com>
>>
>> Fix this warning triggering on a powerpc build:
>> warning: 'error' may be used uninitialized in this function 
>> [-Wmaybe-uninitialized]
>>
>> Signed-off-by: Laurentiu Tudor <laurentiu.tu...@nxp.com>
>> ---
>
> You forgot a reported-by line :(
>
> And I beat you by about 30 minutes to this patch, sorry about that.

Sorry, I guess I'm slow on mondays before having my morning coffee. :-(

> Next time always test-build your patches...

I did but it didn't show up on my version of powerpc toolchain (gcc 
4.8.1). Maybe it's too old ...

---
Best Regards, Laurentiu

Re: [PATCH v4] staging: fsl-mc: move bus driver out of staging

[Laurentiu Tudor]

On 12/19/2017 04:48 PM, Greg KH wrote:
> On Wed, Nov 29, 2017 at 12:08:44PM +0200, laurentiu.tu...@nxp.com wrote:
>> From: Stuart Yoder <stuart.yo...@nxp.com>
>>
>> Move the source files out of staging into their final locations:
>>-include files in drivers/staging/fsl-mc/include go to include/linux/fsl
>>-irq-gic-v3-its-fsl-mc-msi.c goes to drivers/irqchip
>>-source in drivers/staging/fsl-mc/bus goes to drivers/bus/fsl-mc
>>-README.txt, providing and overview of DPAA goes to
>> Documentation/dpaa2/overview.txt
>>
>> Update or delete other remaining staging files-- Makefile, Kconfig, TODO.
>> Update dpaa2_eth and dpio staging drivers.
>>
>> Signed-off-by: Stuart Yoder <stuyo...@gmail.com>
>> Signed-off-by: Laurentiu Tudor <laurentiu.tu...@nxp.com>
>> [Laurentiu: rebased, add dpaa2_eth and dpio #include updates]
>> Cc: Thomas Gleixner <t...@linutronix.de>
>> Cc: Jason Cooper <ja...@lakedaemon.net>
>> Cc: Marc Zyngier <marc.zyng...@arm.com>
>> ---
>> Notes:
>>  -v4:
>>- regenerated patch with renames detection disabled (Andrew Lunn)
>>  -v3:
>>- rebased
>
> Ok, meta-comments on the structure of the code.
>
> You have 8 .h files that are "private" to your bus logic.  That's 7 too
> many, some of them have a bigger license header than actual content :)
>
> Please consolidate into 1.
>
> Also, the headers should be moved to SPDX format to get rid of the
> boilerplate.  I _think_ it's BSD/GPL, right?  Hard to tell :(

It's 3-clause BSD and GPLv2. Will make it clear when moving to SPDX.

>
> Your "public" .h file does not need to go into a subdirectory, just name
> it fsl-mc.h and put it in include/linux/.

There's already a "fsl" subdirectory in include/linux/ so it seemed to 
make sense to use it.

> One comment on the fields in your .h file, all of the user/kernel
> crossing boundry structures need to use the "__" variant of types, like
> "__u8" and the like.  You mix and match them for some reason, you need
> to be consistent.
>
> Also, what's up with the .h files in drivers/staging/fsl-bus/include?
> You didn't touch those with this movement, right?  Why?

Those are not part of the bus "core". Some of them are part of the DPBP 
and DPCON device types APIs and are used by drivers probing on this bus 
and the rest are part of the DPIO driver which is also used by other 
drivers. Since these devices (DPBP, DPCON, DPIO) are interfaces used by 
all the other drivers it made sense to group them together with the bus.

> For this initial move, only move the bus "core" code out, not the other
> stuff like:
>
>>   drivers/irqchip/Makefile   |   1 +
>>   drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c| 119 +++
>
> these should be a separate file move, right?

This bus uses msi interrupts and this file contains glue code needed to 
enable interrupts in the GICv3 irqchip. Without this I don't think the 
bus driver can work because itself makes use of interrupts.

>>   drivers/staging/fsl-dpaa2/ethernet/README  |   2 +-
>
> Why does a README file for a different driver need to be touched?

It mentions a file in the old location of the bus. This is how the diff 
looks:

-   drivers/staging/fsl-mc/README.txt
+   Documentation/dpaa2/overview.txt


>>   drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c |   2 +-
>>   drivers/staging/fsl-dpaa2/ethernet/dpni.c  |   2 +-
>>   drivers/staging/fsl-mc/README.txt  | 386 -
>
> This file gets moved to the Documentation directory, yet it is not tied
> into the documentation build process, that's not good.

Will look into that.

> It doesn't need to have a separate directory either, right?

Agreed, maybe the destination directory isn't the best choice. Maybe 
bus-devices/fsl-mc.txt makes more sense? Can you please suggest?

> And speaking of documentation, you have directories in sysfs, yet no
> Documentation/ABI/ files describing them.  Please fix that up.

Hmm, I was under the impression that we did have sysfs documentation.
Will look into it.

> that's a good start :)

Yep. :)

---
Thanks & Best Regards, Laurentiu

Re: [PATCH v4] staging: fsl-mc: move bus driver out of staging

[Laurentiu Tudor]

On 12/19/2017 05:29 PM, Greg KH wrote:
> On Tue, Dec 19, 2017 at 03:21:19PM +0000, Laurentiu Tudor wrote:
>>
>>
>> On 12/19/2017 04:48 PM, Greg KH wrote:
>>> On Wed, Nov 29, 2017 at 12:08:44PM +0200, laurentiu.tu...@nxp.com wrote:
>>>> From: Stuart Yoder <stuart.yo...@nxp.com>
>>>>
>>>> Move the source files out of staging into their final locations:
>>>> -include files in drivers/staging/fsl-mc/include go to 
>>>> include/linux/fsl
>>>> -irq-gic-v3-its-fsl-mc-msi.c goes to drivers/irqchip
>>>> -source in drivers/staging/fsl-mc/bus goes to drivers/bus/fsl-mc
>>>> -README.txt, providing and overview of DPAA goes to
>>>>  Documentation/dpaa2/overview.txt
>>>>
>>>> Update or delete other remaining staging files-- Makefile, Kconfig, TODO.
>>>> Update dpaa2_eth and dpio staging drivers.
>>>>
>>>> Signed-off-by: Stuart Yoder <stuyo...@gmail.com>
>>>> Signed-off-by: Laurentiu Tudor <laurentiu.tu...@nxp.com>
>>>> [Laurentiu: rebased, add dpaa2_eth and dpio #include updates]
>>>> Cc: Thomas Gleixner <t...@linutronix.de>
>>>> Cc: Jason Cooper <ja...@lakedaemon.net>
>>>> Cc: Marc Zyngier <marc.zyng...@arm.com>
>>>> ---
>>>> Notes:
>>>>   -v4:
>>>> - regenerated patch with renames detection disabled (Andrew Lunn)
>>>>   -v3:
>>>> - rebased
>>>
>>> Ok, meta-comments on the structure of the code.
>>>
>>> You have 8 .h files that are "private" to your bus logic.  That's 7 too
>>> many, some of them have a bigger license header than actual content :)
>>>
>>> Please consolidate into 1.
>>>
>>> Also, the headers should be moved to SPDX format to get rid of the
>>> boilerplate.  I _think_ it's BSD/GPL, right?  Hard to tell :(
>>
>> It's 3-clause BSD and GPLv2. Will make it clear when moving to SPDX.
>
> Thanks.
>
>>> Your "public" .h file does not need to go into a subdirectory, just name
>>> it fsl-mc.h and put it in include/linux/.
>>
>> There's already a "fsl" subdirectory in include/linux/ so it seemed to
>> make sense to use it.
>
> Ah, missed that.  Ok, nevermind :)`
>
>>> One comment on the fields in your .h file, all of the user/kernel
>>> crossing boundry structures need to use the "__" variant of types, like
>>> "__u8" and the like.  You mix and match them for some reason, you need
>>> to be consistent.
>>>
>>> Also, what's up with the .h files in drivers/staging/fsl-bus/include?
>>> You didn't touch those with this movement, right?  Why?
>>
>> Those are not part of the bus "core". Some of them are part of the DPBP
>> and DPCON device types APIs and are used by drivers probing on this bus
>> and the rest are part of the DPIO driver which is also used by other
>> drivers. Since these devices (DPBP, DPCON, DPIO) are interfaces used by
>> all the other drivers it made sense to group them together with the bus.
>
> But all of these .h files are only used by the code in this specific
> directory, no where else.

They are also used by our ethernet driver, see:
   drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h

> So just mush them all together, having
> individual .h files doesn't really help anyone here, right?  It's just
> more files for no good reason.  And, it might help you see if you really
> need all of the info in those files :)

Ok, I'll try to come up with something that reduces the number of header 
files.

>>> For this initial move, only move the bus "core" code out, not the other
>>> stuff like:
>>>
>>>>drivers/irqchip/Makefile   |   1 +
>>>>drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c| 119 +++
>>>
>>> these should be a separate file move, right?
>>
>> This bus uses msi interrupts and this file contains glue code needed to
>> enable interrupts in the GICv3 irqchip. Without this I don't think the
>> bus driver can work because itself makes use of interrupts.
>
> How is this all working today?  Just leave the non-bus code alone, and
> move the irqchip code as a separate patch.

Ok, i can do this.

>>>>drivers/staging/fsl-dpaa2/ethernet/README  |   2 +-
>>>
>>> Why does a README file for a different driver need to be touched?
>>
>> It mentions a file in the old location of the 

Re: [PATCH v4] staging: fsl-mc: move bus driver out of staging

[Laurentiu Tudor]

On 12/19/2017 06:10 PM, Greg KH wrote:
> On Tue, Dec 19, 2017 at 03:39:44PM +0000, Laurentiu Tudor wrote:
>> On 12/19/2017 05:29 PM, Greg KH wrote:
>>> On Tue, Dec 19, 2017 at 03:21:19PM +0000, Laurentiu Tudor wrote:
>>>>
>>>>
>>>> On 12/19/2017 04:48 PM, Greg KH wrote:
>>>>> On Wed, Nov 29, 2017 at 12:08:44PM +0200, laurentiu.tu...@nxp.com wrote:
>>>>>> From: Stuart Yoder <stuart.yo...@nxp.com>
>>>>>>
>>>>>> Move the source files out of staging into their final locations:
>>>>>>  -include files in drivers/staging/fsl-mc/include go to 
>>>>>> include/linux/fsl
>>>>>>  -irq-gic-v3-its-fsl-mc-msi.c goes to drivers/irqchip
>>>>>>  -source in drivers/staging/fsl-mc/bus goes to drivers/bus/fsl-mc
>>>>>>  -README.txt, providing and overview of DPAA goes to
>>>>>>   Documentation/dpaa2/overview.txt
>>>>>>
>>>>>> Update or delete other remaining staging files-- Makefile, Kconfig, TODO.
>>>>>> Update dpaa2_eth and dpio staging drivers.
>>>>>>
>>>>>> Signed-off-by: Stuart Yoder <stuyo...@gmail.com>
>>>>>> Signed-off-by: Laurentiu Tudor <laurentiu.tu...@nxp.com>
>>>>>> [Laurentiu: rebased, add dpaa2_eth and dpio #include updates]
>>>>>> Cc: Thomas Gleixner <t...@linutronix.de>
>>>>>> Cc: Jason Cooper <ja...@lakedaemon.net>
>>>>>> Cc: Marc Zyngier <marc.zyng...@arm.com>
>>>>>> ---
>>>>>> Notes:
>>>>>>-v4:
>>>>>>  - regenerated patch with renames detection disabled (Andrew 
>>>>>> Lunn)
>>>>>>-v3:
>>>>>>  - rebased
>>>>>
>>>>> Ok, meta-comments on the structure of the code.
>>>>>
>>>>> You have 8 .h files that are "private" to your bus logic.  That's 7 too
>>>>> many, some of them have a bigger license header than actual content :)
>>>>>
>>>>> Please consolidate into 1.
>>>>>
>>>>> Also, the headers should be moved to SPDX format to get rid of the
>>>>> boilerplate.  I _think_ it's BSD/GPL, right?  Hard to tell :(
>>>>
>>>> It's 3-clause BSD and GPLv2. Will make it clear when moving to SPDX.
>>>
>>> Thanks.
>>>
>>>>> Your "public" .h file does not need to go into a subdirectory, just name
>>>>> it fsl-mc.h and put it in include/linux/.
>>>>
>>>> There's already a "fsl" subdirectory in include/linux/ so it seemed to
>>>> make sense to use it.
>>>
>>> Ah, missed that.  Ok, nevermind :)`
>>>
>>>>> One comment on the fields in your .h file, all of the user/kernel
>>>>> crossing boundry structures need to use the "__" variant of types, like
>>>>> "__u8" and the like.  You mix and match them for some reason, you need
>>>>> to be consistent.
>>>>>
>>>>> Also, what's up with the .h files in drivers/staging/fsl-bus/include?
>>>>> You didn't touch those with this movement, right?  Why?
>>>>
>>>> Those are not part of the bus "core". Some of them are part of the DPBP
>>>> and DPCON device types APIs and are used by drivers probing on this bus
>>>> and the rest are part of the DPIO driver which is also used by other
>>>> drivers. Since these devices (DPBP, DPCON, DPIO) are interfaces used by
>>>> all the other drivers it made sense to group them together with the bus.
>>>
>>> But all of these .h files are only used by the code in this specific
>>> directory, no where else.
>>
>> They are also used by our ethernet driver, see:
>> drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
>
> Ick, really?  Then they should not be buried in a bus-specific
> location, but rather be in include/linux/SOMEWHERE, right?

Right. The goal is that in the end, all headers be moved to the already 
existing include/linux/fsl/. For now I've left these in staging because 
they are not part of the bus "core" infrastructure.

---
Best Regards, Laurentiu

Re: [PATCH v4] staging: fsl-mc: move bus driver out of staging

[Laurentiu Tudor]

On 12/20/2017 12:42 PM, Greg KH wrote:
> On Wed, Dec 20, 2017 at 10:26:49AM +0000, Laurentiu Tudor wrote:
>> On 12/19/2017 06:10 PM, Greg KH wrote:
>>>>> But all of these .h files are only used by the code in this specific
>>>>> directory, no where else.
>>>>
>>>> They are also used by our ethernet driver, see:
>>>>  drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
>>>
>>> Ick, really?  Then they should not be buried in a bus-specific
>>> location, but rather be in include/linux/SOMEWHERE, right?
>>
>> Right. The goal is that in the end, all headers be moved to the already
>> existing include/linux/fsl/. For now I've left these in staging because
>> they are not part of the bus "core" infrastructure.
>
> Then shouldn't they be in the drivers/staging/fsl-mc/include/ directory
> now to show this?

Not sure i get your comment. Aren't we talking about the headers in there?

This was your original comment:

 > Also, what's up with the .h files in drivers/staging/fsl-bus/include?
 > You didn't touch those with this movement, right?  Why?

---
Best Regards, Laurentiu

Re: [PATCH v4] staging: fsl-mc: move bus driver out of staging

[Laurentiu Tudor]

On 12/20/2017 01:06 PM, Greg KH wrote:
> On Wed, Dec 20, 2017 at 10:52:52AM +0000, Laurentiu Tudor wrote:
>>
>>
>> On 12/20/2017 12:42 PM, Greg KH wrote:
>>> On Wed, Dec 20, 2017 at 10:26:49AM +, Laurentiu Tudor wrote:
>>>> On 12/19/2017 06:10 PM, Greg KH wrote:
>>>>>>> But all of these .h files are only used by the code in this specific
>>>>>>> directory, no where else.
>>>>>>
>>>>>> They are also used by our ethernet driver, see:
>>>>>>   drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
>>>>>
>>>>> Ick, really?  Then they should not be buried in a bus-specific
>>>>> location, but rather be in include/linux/SOMEWHERE, right?
>>>>
>>>> Right. The goal is that in the end, all headers be moved to the already
>>>> existing include/linux/fsl/. For now I've left these in staging because
>>>> they are not part of the bus "core" infrastructure.
>>>
>>> Then shouldn't they be in the drivers/staging/fsl-mc/include/ directory
>>> now to show this?
>>
>> Not sure i get your comment. Aren't we talking about the headers in there?
>>
>> This was your original comment:
>>
>>   > Also, what's up with the .h files in drivers/staging/fsl-bus/include?
>>   > You didn't touch those with this movement, right?  Why?
>
> Ok, yeah, I'm getting confused now.  Let's just see what you do with
> your next set of patches and we can go from there :)

Ok, I'll start working on it. Thanks for the review!

---
Best Regards, Laurentiu

Re: [PATCH v2] staging: fsl-mc: move bus driver out of staging

[Laurentiu Tudor]

Hi Greg,

On 11/03/2017 05:17 PM, Greg KH wrote:
> On Thu, Aug 31, 2017 at 06:04:30PM +0200, Greg KH wrote:
>> On Mon, Aug 28, 2017 at 01:54:05PM +0300, laurentiu.tu...@nxp.com wrote:
>>> From: Stuart Yoder <stuart.yo...@nxp.com>
>>>
>>> Move the source files out of staging into their final locations:
>>>-include files in drivers/staging/fsl-mc/include go to include/linux/fsl
>>>-irq-gic-v3-its-fsl-mc-msi.c goes to drivers/irqchip
>>>-source in drivers/staging/fsl-mc/bus goes to drivers/bus/fsl-mc
>>>-README.txt, providing and overview of DPAA goes to
>>> Documentation/dpaa2/overview.txt
>>>
>>> Update or delete other remaining staging files-- Makefile, Kconfig, TODO.
>>> Update dpaa2_eth and dpio staging drivers.
>>>
>>> Signed-off-by: Stuart Yoder <stuyo...@gmail.com>
>>> Signed-off-by: Laurentiu Tudor <laurentiu.tu...@nxp.com>
>>> [Laurentiu: rebased, add dpaa2_eth and dpio #include updates]
>>> Cc: Thomas Gleixner <t...@linutronix.de>
>>> Cc: Jason Cooper <ja...@lakedaemon.net>
>>> Cc: Marc Zyngier <marc.zyng...@arm.com>
>>
>> This is going to have to wait until I get a chunk of time to do the
>> review.  Probably after 4.13-final is out.
>
> Ok, review comments as I go through the code:

Thanks a lot for taking the time. I'll take care of the comments next week.

---
Best Regards, Laurentiu

Re: [PATCH v2] staging: fsl-mc: move bus driver out of staging

[Laurentiu Tudor]

On 11/03/2017 05:17 PM, Greg KH wrote:
> On Thu, Aug 31, 2017 at 06:04:30PM +0200, Greg KH wrote:
>> On Mon, Aug 28, 2017 at 01:54:05PM +0300, laurentiu.tu...@nxp.com wrote:
>>> From: Stuart Yoder <stuart.yo...@nxp.com>
>>>
>>> Move the source files out of staging into their final locations:
>>>-include files in drivers/staging/fsl-mc/include go to include/linux/fsl
>>>-irq-gic-v3-its-fsl-mc-msi.c goes to drivers/irqchip
>>>-source in drivers/staging/fsl-mc/bus goes to drivers/bus/fsl-mc
>>>-README.txt, providing and overview of DPAA goes to
>>> Documentation/dpaa2/overview.txt
>>>
>>> Update or delete other remaining staging files-- Makefile, Kconfig, TODO.
>>> Update dpaa2_eth and dpio staging drivers.
>>>
>>> Signed-off-by: Stuart Yoder <stuyo...@gmail.com>
>>> Signed-off-by: Laurentiu Tudor <laurentiu.tu...@nxp.com>
>>> [Laurentiu: rebased, add dpaa2_eth and dpio #include updates]
>>> Cc: Thomas Gleixner <t...@linutronix.de>
>>> Cc: Jason Cooper <ja...@lakedaemon.net>
>>> Cc: Marc Zyngier <marc.zyng...@arm.com>
>>
>> This is going to have to wait until I get a chunk of time to do the
>> review.  Probably after 4.13-final is out.
>
> Ok, review comments as I go through the code:
>   mc-sys.c 323 EXPORT_SYMBOL(mc_send_command);
>
> should be EXPORT_SYMBOL_GPL(fsl_mc_send_command); to match up with your
> other exports and global namespace, right?

Right. There's an inconsistent mixture of EXPORT_SYMBOL() and 
EXPORT_SYMBOL_GPL() usage. I'll change them all to EXPORT_SYMBOL_GPL().

> You export a lot of dpcon_* symbols that no one uses, please do not do
> that.  Verify that all of them are actually used right now, if not,
> remove them.  If you think you are going to use them in the future,
> wonderful, add them in then.

Actually, most of the dpcon_* APIs are used in the ethernet driver here: 
drivers/staging/fsl-dpaa2/ethernet. I think i saw only a couple of 
functions that are not used so I'll drop those.

> Same for your dpaa2_* exported symbols, most are not used from what I
> can see.

I'll check these too and drop the unused ones.

> struct dpaa2_io {
>  atomic_t refs;
>
> That's a kref, please use it instead of trying to roll your own.
>
> And even for this, your locking is not correct (i.e. you do not have
> any), that needs to be fixed so that teardown works correctly.

I think we can drop this refcount altogether as it's not used. Roy, any 
comment on this?

> You have a lot of WARN_ON() calls, that's going to be ignored and should
> all not be needed now that the code is debugged and working properly.
> Please remove them, or turn them into dev_err() calls with a real if ()
> check instead.

Right, there are quite of few (100+) WARN_ON()s. I'll go through them 
and clean them up ... most of them seem paranoid checks anyway.

> You are checking "strings" for the type of device in a lot of places,
> like this:
>   if (strcmp(obj_desc->type, "dprc") == 0) {
> why are you not just using the built-in driver model .type field and
> comparing that to the different type structures?  It's much easier, and
> you don't have to again, "roll your own".  See the USB or Greybus code
> for examples of busses that have different "types" of devices on them at
> the same time.
>

I had a quick look over greybus and noticed the device types declared in 
drivers/staging/greybus/greybus.h plus some helper macros to check the 
device type. I'll give this a go, though i might return with some 
questions (e.g. I don't yet understand what device_type::release op is 
used for).

---
Thanks & Best Regards, Laurentiu

Re: [PATCH 1/2] staging: fsl-mc: Allocate IRQ's before scanning DPRC objects

[Laurentiu Tudor]

On 12/07/2017 03:18 PM, Nipun Gupta wrote:
>
>
>> -Original Message-----
>> From: Laurentiu Tudor
>> Sent: Wednesday, December 06, 2017 19:00
>> To: Nipun Gupta <nipun.gu...@nxp.com>; stuyo...@gmail.com; Bharat
>> Bhushan <bharat.bhus...@nxp.com>; gre...@linuxfoundation.org;
>> cakt...@gmail.com; bretth...@gmail.com; a...@arndb.de
>> Cc: linux-kernel@vger.kernel.org; de...@driverdev.osuosl.org
>> Subject: Re: [PATCH 1/2] staging: fsl-mc: Allocate IRQ's before scanning DPRC
>> objects
>>
>> Hi Nipun,
>>
>> Can you polish a bit this commit message? It doesn't seem to explain why
>> this is needed.
>
> Sure. Ill update the message.
>
>>
>> On 12/06/2017 06:18 PM, Nipun Gupta wrote:
>>> When DPRC probing is deferred (such as where IOMMU is not probed
>>> before the fsl-mc bus), all the devices in the DPRC containers gets
>>> initialized one after another.
>>
>> Are you referring to dprc probing being deferred (do we ever do that?)
>> or devices inside the dprc deferring the probe?
>
> Yes.. Currently following is the scenario when the devices are probed
> (please correct me if I am wrong):
>
>   FSL_MC Bus probe ---> dprc probe ---> dprc devices scan --->
>   probe of devices in DPRC container ---> allocate IRQ's.
>
> In case the devices being probed in the DPRC container need the IRQ's;
> probing of that device will fail.
> In the current scenario DPIO device while getting probed for the first time
> gets deferred because the DPIO driver is not yet registered.
> So there is no impact seen in the current code.
>
> In case the DPRC probing gets deferred, does in case IOMMU is enabled
> (though it is not enabled till now for fsl-mc bus but we plan to add the
> support as soon as mc bus is out from staging); the DPIO gets probed
> before IRQ's being allocated. This causes DPIO probe to fail.
>
> So I think it makes sense that IRQ's are allocated before any devices
> In the DPRC container are probed.

Thanks for the details. It would be great if you could update the commit 
message with these execution flow details.

>>
>>> As IRQ's were allocated only once the
>>> DPRC scanning is completed, the devices like DPIO which uses these
>>> IRQ's at initalization fails. This patch allocates the IRQ resources
>>
>> s/initalization/initialization
>>
>>> before scanning all the objects in the DPRC container.
>>>
>>> Signed-off-by: Nipun Gupta <nipun.gu...@nxp.com>
>>> ---
>>>drivers/staging/fsl-mc/bus/dprc-driver.c | 49 
>>> ++
>> --
>>>1 file changed, 27 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c b/drivers/staging/fsl-
>> mc/bus/dprc-driver.c
>>> index 06df528..7265431 100644
>>> --- a/drivers/staging/fsl-mc/bus/dprc-driver.c
>>> +++ b/drivers/staging/fsl-mc/bus/dprc-driver.c
>>> @@ -206,7 +206,8 @@ static void dprc_add_new_devices(struct
>> fsl_mc_device *mc_bus_dev,
>>> * dprc_scan_objects - Discover objects in a DPRC
>>> *
>>> * @mc_bus_dev: pointer to the fsl-mc device that represents a DPRC 
>>> object
>>> - * @total_irq_count: total number of IRQs needed by objects in the DPRC.
>>> + * @total_irq_count: If argument is provided the function populates the
>>> + * total number of IRQs created by objects in the DPRC.
>>
>> As a side node, after a cursory look i noticed that this total_irq_count
>> parameter is used only for some sanity checks. I'm thinking of dropping
>> it in a follow-up cleanup patch.
>
> Do you plan to send it very recently.
> In that case I can rebase my patch on top of it.

It's not on my short term TODO.

---
Best Regards, Laurentiu

Re: [PATCH 1/2] staging: fsl-mc: Allocate IRQ's before scanning DPRC objects

[Laurentiu Tudor]

Hi Nipun,

Can you polish a bit this commit message? It doesn't seem to explain why 
this is needed.

On 12/06/2017 06:18 PM, Nipun Gupta wrote:
> When DPRC probing is deferred (such as where IOMMU is not probed
> before the fsl-mc bus), all the devices in the DPRC containers gets
> initialized one after another.

Are you referring to dprc probing being deferred (do we ever do that?) 
or devices inside the dprc deferring the probe?

> As IRQ's were allocated only once the
> DPRC scanning is completed, the devices like DPIO which uses these
> IRQ's at initalization fails. This patch allocates the IRQ resources

s/initalization/initialization

> before scanning all the objects in the DPRC container.
>
> Signed-off-by: Nipun Gupta 
> ---
>   drivers/staging/fsl-mc/bus/dprc-driver.c | 49 
> ++--
>   1 file changed, 27 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c 
> b/drivers/staging/fsl-mc/bus/dprc-driver.c
> index 06df528..7265431 100644
> --- a/drivers/staging/fsl-mc/bus/dprc-driver.c
> +++ b/drivers/staging/fsl-mc/bus/dprc-driver.c
> @@ -206,7 +206,8 @@ static void dprc_add_new_devices(struct fsl_mc_device 
> *mc_bus_dev,
>* dprc_scan_objects - Discover objects in a DPRC
>*
>* @mc_bus_dev: pointer to the fsl-mc device that represents a DPRC object
> - * @total_irq_count: total number of IRQs needed by objects in the DPRC.
> + * @total_irq_count: If argument is provided the function populates the
> + * total number of IRQs created by objects in the DPRC.

As a side node, after a cursory look i noticed that this total_irq_count 
parameter is used only for some sanity checks. I'm thinking of dropping 
it in a follow-up cleanup patch.

---
Best Regards, Laurentiu

Re: [PATCH 1/2] staging: fsl-mc: Allocate IRQ's before scanning DPRC objects

[Laurentiu Tudor]

Hi Bharat,

On 12/06/2017 04:03 PM, Bharat Bhushan wrote:
> Hi Laurentiu,
>
>> -Original Message-----
>> From: Laurentiu Tudor
>> Sent: Wednesday, December 06, 2017 7:00 PM
>> To: Nipun Gupta <nipun.gu...@nxp.com>; stuyo...@gmail.com; Bharat
>> Bhushan <bharat.bhus...@nxp.com>; gre...@linuxfoundation.org;
>> cakt...@gmail.com; bretth...@gmail.com; a...@arndb.de
>> Cc: linux-kernel@vger.kernel.org; de...@driverdev.osuosl.org
>> Subject: Re: [PATCH 1/2] staging: fsl-mc: Allocate IRQ's before scanning DPRC
>> objects
>>
>> Hi Nipun,
>>
>> Can you polish a bit this commit message? It doesn't seem to explain why this
>> is needed.
>>
>> On 12/06/2017 06:18 PM, Nipun Gupta wrote:
>>> When DPRC probing is deferred (such as where IOMMU is not probed
>>> before the fsl-mc bus), all the devices in the DPRC containers gets
>>> initialized one after another.
>>
>> Are you referring to dprc probing being deferred (do we ever do that?) or
>> devices inside the dprc deferring the probe?
>>
>>> As IRQ's were allocated only once the
>>> DPRC scanning is completed, the devices like DPIO which uses these
>>> IRQ's at initalization fails. This patch allocates the IRQ resources
>>
>> s/initalization/initialization
>>
>>> before scanning all the objects in the DPRC container.
>>>
>>> Signed-off-by: Nipun Gupta <nipun.gu...@nxp.com>
>>> ---
>>>drivers/staging/fsl-mc/bus/dprc-driver.c | 49 ++--
>> 
>>>1 file changed, 27 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c
>>> b/drivers/staging/fsl-mc/bus/dprc-driver.c
>>> index 06df528..7265431 100644
>>> --- a/drivers/staging/fsl-mc/bus/dprc-driver.c
>>> +++ b/drivers/staging/fsl-mc/bus/dprc-driver.c
>>> @@ -206,7 +206,8 @@ static void dprc_add_new_devices(struct
>> fsl_mc_device *mc_bus_dev,
>>> * dprc_scan_objects - Discover objects in a DPRC
>>> *
>>> * @mc_bus_dev: pointer to the fsl-mc device that represents a DPRC
>>> object
>>> - * @total_irq_count: total number of IRQs needed by objects in the DPRC.
>>> + * @total_irq_count: If argument is provided the function populates
>>> + the
>>> + * total number of IRQs created by objects in the DPRC.
>>
>> As a side node, after a cursory look i noticed that this total_irq_count
>> parameter is used only for some sanity checks. I'm thinking of dropping it 
>> in a
>> follow-up cleanup patch.
>
> How you will ensure that number of IRQ needed are not sufficient for devices 
> in the container?
> Do you think we need to either not enable additional devices or add irqs to 
> irq-pool ?

In the current implementation we allocate a pool of 
FSL_MC_IRQ_POOL_MAX_TOTAL_IRQS (= 256) no mater what total_irq_count is.
I think this is enough for our current scenarios, but if in the future 
we ever hit this limit we can think of a mechanism along the lines of 
your example. Adding another chunk of irqs to the pool seems to me like 
a good idea in the future.

---
Best Regards, Laurentiu

Re: [PATCH 5/6 v3] bus: fsl-mc: supoprt dma configure for devices on fsl-mc bus

[Laurentiu Tudor]

Hi Nipun,

On 04/27/2018 01:27 PM, Nipun Gupta wrote:
> Signed-off-by: Nipun Gupta 
> ---
>   drivers/bus/fsl-mc/fsl-mc-bus.c | 16 
>   1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c
> index 5d8266c..624828b 100644
> --- a/drivers/bus/fsl-mc/fsl-mc-bus.c
> +++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
> @@ -127,6 +127,16 @@ static int fsl_mc_bus_uevent(struct device *dev, struct 
> kobj_uevent_env *env)
>   return 0;
>   }
>
> +static int fsl_mc_dma_configure(struct device *dev)
> +{
> + struct device *dma_dev = dev;
> +
> + while (dev_is_fsl_mc(dma_dev))
> + dma_dev = dma_dev->parent;
> +
> + return of_dma_configure(dev, dma_dev->of_node, 0);
> +}
> +
>   static ssize_t modalias_show(struct device *dev, struct device_attribute 
> *attr,
>char *buf)
>   {
> @@ -148,6 +158,7 @@ struct bus_type fsl_mc_bus_type = {
>   .name = "fsl-mc",
>   .match = fsl_mc_bus_match,
>   .uevent = fsl_mc_bus_uevent,
> + .dma_configure  = fsl_mc_dma_configure,
>   .dev_groups = fsl_mc_dev_groups,
>   };
>   EXPORT_SYMBOL_GPL(fsl_mc_bus_type);
> @@ -616,6 +627,7 @@ int fsl_mc_device_add(struct fsl_mc_obj_desc *obj_desc,
>   mc_dev->icid = parent_mc_dev->icid;
>   mc_dev->dma_mask = FSL_MC_DEFAULT_DMA_MASK;
>   mc_dev->dev.dma_mask = _dev->dma_mask;
> + mc_dev->dev.coherent_dma_mask = mc_dev->dma_mask;

This change seems a bit unrelated to the patch subject. I wonder if it 
makes sense to split it it in a distinct patch.

---
Best Regards, Laurentiu

Re: [PATCH 6/7 v5] bus: fsl-mc: set coherent dma mask for devices on fsl-mc bus

[Laurentiu Tudor]

On 05/20/2018 04:49 PM, Nipun Gupta wrote:
> of_dma_configure() API expects coherent_dma_mask to be correctly
> set in the devices. This patch does the needful.
>
> Signed-off-by: Nipun Gupta <nipun.gu...@nxp.com>

Acked-by: Laurentiu Tudor <laurentiu.tu...@nxp.com>

---
Best Regards, Laurentiu

> ---
>   drivers/bus/fsl-mc/fsl-mc-bus.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c
> index fa43c7d..624828b 100644
> --- a/drivers/bus/fsl-mc/fsl-mc-bus.c
> +++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
> @@ -627,6 +627,7 @@ int fsl_mc_device_add(struct fsl_mc_obj_desc *obj_desc,
>   mc_dev->icid = parent_mc_dev->icid;
>   mc_dev->dma_mask = FSL_MC_DEFAULT_DMA_MASK;
>   mc_dev->dev.dma_mask = _dev->dma_mask;
> + mc_dev->dev.coherent_dma_mask = mc_dev->dma_mask;
>   dev_set_msi_domain(_dev->dev,
>  dev_get_msi_domain(_mc_dev->dev));
>   }
>

[PATCH] irqchip/ls-scfg-msi: map MSIs in the iommu

[Laurentiu Tudor]

Add the required iommu_dma_map_msi_msg() when composing the MSI message,
otherwise the interrupts will not work.

Signed-off-by: Laurentiu Tudor 
---
 drivers/irqchip/irq-ls-scfg-msi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/irqchip/irq-ls-scfg-msi.c 
b/drivers/irqchip/irq-ls-scfg-msi.c
index 57e3d900f19e..1ec3bfe56693 100644
--- a/drivers/irqchip/irq-ls-scfg-msi.c
+++ b/drivers/irqchip/irq-ls-scfg-msi.c
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #define MSI_IRQS_PER_MSIR  32
 #define MSI_MSIR_OFFSET4
@@ -94,6 +95,8 @@ static void ls_scfg_msi_compose_msg(struct irq_data *data, 
struct msi_msg *msg)
 
if (msi_affinity_flag)
msg->data |= cpumask_first(data->common->affinity);
+
+   iommu_dma_map_msi_msg(data->irq, msg);
 }
 
 static int ls_scfg_msi_set_affinity(struct irq_data *irq_data,
-- 
2.17.0

Re: [PATCH] irqchip/ls-scfg-msi: update effective affinity mask

[Laurentiu Tudor]

Hi Marc,

On 26.06.2018 17:56, Marc Zyngier wrote:
> On Tue, 26 Jun 2018 15:37:12 +0100,
> Laurentiu Tudor  wrote:
>>
>> Update the effective affinity mask to fix this warning issued by the
>> generic irq handling code:
>>
>> "genirq: irq_chip MSI did not update eff. affinity mask of irq x"
>>
>> Signed-off-by: Laurentiu Tudor 
>> ---
>>   drivers/irqchip/irq-ls-scfg-msi.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/irqchip/irq-ls-scfg-msi.c 
>> b/drivers/irqchip/irq-ls-scfg-msi.c
>> index 1ec3bfe56693..d81d5b016438 100644
>> --- a/drivers/irqchip/irq-ls-scfg-msi.c
>> +++ b/drivers/irqchip/irq-ls-scfg-msi.c
>> @@ -122,6 +122,7 @@ static int ls_scfg_msi_set_affinity(struct irq_data 
>> *irq_data,
>>  }
>>   
>>  cpumask_copy(irq_data->common->affinity, mask);
>> +irq_data_update_effective_affinity(irq_data, mask);
>>   
>>  return IRQ_SET_MASK_OK;
>>   }
> 
> It has already been addressed here:
> 

Thanks for the obviously much better & complete fix.
Will take care to keep my local repo more up-to-date.

---
Best Regards, Laurentiu

[PATCH] irqchip/ls-scfg-msi: update effective affinity mask

[Laurentiu Tudor]

Update the effective affinity mask to fix this warning issued by the
generic irq handling code:

"genirq: irq_chip MSI did not update eff. affinity mask of irq x"

Signed-off-by: Laurentiu Tudor 
---
 drivers/irqchip/irq-ls-scfg-msi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/irqchip/irq-ls-scfg-msi.c 
b/drivers/irqchip/irq-ls-scfg-msi.c
index 1ec3bfe56693..d81d5b016438 100644
--- a/drivers/irqchip/irq-ls-scfg-msi.c
+++ b/drivers/irqchip/irq-ls-scfg-msi.c
@@ -122,6 +122,7 @@ static int ls_scfg_msi_set_affinity(struct irq_data 
*irq_data,
}
 
cpumask_copy(irq_data->common->affinity, mask);
+   irq_data_update_effective_affinity(irq_data, mask);
 
return IRQ_SET_MASK_OK;
 }
-- 
2.17.1

Re: [PATCH] mmc: sdhci-of-esdhc: set proper dma mask for ls104x chips

[Laurentiu Tudor]

Hi Uffe,

On 02.07.2018 17:37, Ulf Hansson wrote:
> On 28 June 2018 at 10:45, Laurentiu Tudor  wrote:
>> SDHCI controller in ls1043a and ls1046a generate 40-bit wide addresses
>> when doing DMA. Make sure that the corresponding dma mask is correctly
>> configured.
>>
>> Signed-off-by: Laurentiu Tudor 
> 
> Thanks, applied for next!

Thanks for picking this up!
I just realized that maybe I should have provided some context around 
this patch. I'm working on enabling smmu on these chips and encountered 
the following problem: the smmu input address size is 48 bits so the dma 
mappings for sdhci end up 48-bit wide. However, on these chips sdhci 
only use 40-bits of that address size when doing dma.
So you end up with a 48-bit address translation in smmu but the device 
generates transactions with clipped 40-bit addresses, thus smmu context 
faults are triggered. Setting up the correct dma mask fixes this situation.

---
Best Regards, Laurentiu

> 
>> ---
>>   drivers/mmc/host/sdhci-of-esdhc.c | 6 ++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/mmc/host/sdhci-of-esdhc.c 
>> b/drivers/mmc/host/sdhci-of-esdhc.c
>> index 4ffa6b173a21..8332f56e6c0d 100644
>> --- a/drivers/mmc/host/sdhci-of-esdhc.c
>> +++ b/drivers/mmc/host/sdhci-of-esdhc.c
>> @@ -22,6 +22,7 @@
>>   #include 
>>   #include 
>>   #include 
>> +#include 
>>   #include 
>>   #include "sdhci-pltfm.h"
>>   #include "sdhci-esdhc.h"
>> @@ -427,6 +428,11 @@ static void esdhc_of_adma_workaround(struct sdhci_host 
>> *host, u32 intmask)
>>   static int esdhc_of_enable_dma(struct sdhci_host *host)
>>   {
>>  u32 value;
>> +   struct device *dev = mmc_dev(host->mmc);
>> +
>> +   if (of_device_is_compatible(dev->of_node, "fsl,ls1043a-esdhc") ||
>> +   of_device_is_compatible(dev->of_node, "fsl,ls1046a-esdhc"))
>> +   dma_set_mask_and_coherent(dev, DMA_BIT_MASK(40));
>>
>>  value = sdhci_readl(host, ESDHC_DMA_SYSCTL);
>>  value |= ESDHC_DMA_SNOOP;
>> --
>> 2.17.1
>>

Re: [PATCH v4 6/6] arm64: dts: ls208xa: comply with the iommu map binding for fsl_mc

[Laurentiu Tudor]

Hi Nipun,

On 04/30/2018 09:27 AM, Nipun Gupta wrote:
> fsl-mc bus support the new iommu-map property. Comply to this binding
> for fsl_mc bus.
>
> Signed-off-by: Nipun Gupta <nipun.gu...@nxp.com>

This looks good to me, so:

Reviewed-By: Laurentiu Tudor <laurentiu.tu...@nxp.com>

---
Best Regards, Laurentiu

> ---
>   arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi | 6 +-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi 
> b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
> index 137ef4d..6010505 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
> @@ -184,6 +184,7 @@
>   #address-cells = <2>;
>   #size-cells = <2>;
>   ranges;
> + dma-ranges = <0x0 0x0 0x0 0x0 0x1 0x>;
>
>   clockgen: clocking@130 {
>   compatible = "fsl,ls2080a-clockgen";
> @@ -357,6 +358,8 @@
>   reg = <0x0008 0x0c00 0 0x40>,/* MC portal 
> base */
> <0x 0x0834 0 0x4>; /* MC control 
> reg */
>   msi-parent = <>;
> + iommu-map = <0  0 0>;  /* This is fixed-up by 
> u-boot */
> + dma-coherent;
>   #address-cells = <3>;
>   #size-cells = <1>;
>
> @@ -460,6 +463,8 @@
>   compatible = "arm,mmu-500";
>   reg = <0 0x500 0 0x80>;
>   #global-interrupts = <12>;
> + #iommu-cells = <1>;
> + stream-match-mask = <0x7C00>;
>   interrupts = <0 13 4>, /* global secure fault */
><0 14 4>, /* combined secure interrupt */
><0 15 4>, /* global non-secure fault */
> @@ -502,7 +507,6 @@
><0 204 4>, <0 205 4>,
><0 206 4>, <0 207 4>,
><0 208 4>, <0 209 4>;
> - mmu-masters = <_mc 0x300 0>;
>   };
>
>   dspi: dspi@210 {
>

Re: [PATCH v4 5/6] bus: fsl-mc: supoprt dma configure for devices on fsl-mc bus

[Laurentiu Tudor]

Hi Nipun,

On 04/30/2018 09:27 AM, Nipun Gupta wrote:
> Signed-off-by: Nipun Gupta <nipun.gu...@nxp.com>
> ---

If my understanding is correct, the kbuild error is triggered by this 
missing dependency patch:

https://patchwork.kernel.org/patch/10370081/

Apart from that, patch looks good to me, so

Reviewed-by: Laurentiu Tudor <laurentiu.tu...@nxp.com>

---
Best Regards, Laurentiu

>   drivers/bus/fsl-mc/fsl-mc-bus.c | 16 
>   1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c
> index 5d8266c..624828b 100644
> --- a/drivers/bus/fsl-mc/fsl-mc-bus.c
> +++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
> @@ -127,6 +127,16 @@ static int fsl_mc_bus_uevent(struct device *dev, struct 
> kobj_uevent_env *env)
>   return 0;
>   }
>
> +static int fsl_mc_dma_configure(struct device *dev)
> +{
> + struct device *dma_dev = dev;
> +
> + while (dev_is_fsl_mc(dma_dev))
> + dma_dev = dma_dev->parent;
> +
> + return of_dma_configure(dev, dma_dev->of_node, 0);
> +}
> +
>   static ssize_t modalias_show(struct device *dev, struct device_attribute 
> *attr,
>char *buf)
>   {
> @@ -148,6 +158,7 @@ struct bus_type fsl_mc_bus_type = {
>   .name = "fsl-mc",
>   .match = fsl_mc_bus_match,
>   .uevent = fsl_mc_bus_uevent,
> + .dma_configure  = fsl_mc_dma_configure,
>   .dev_groups = fsl_mc_dev_groups,
>   };
>   EXPORT_SYMBOL_GPL(fsl_mc_bus_type);
> @@ -616,6 +627,7 @@ int fsl_mc_device_add(struct fsl_mc_obj_desc *obj_desc,
>   mc_dev->icid = parent_mc_dev->icid;
>   mc_dev->dma_mask = FSL_MC_DEFAULT_DMA_MASK;
>   mc_dev->dev.dma_mask = _dev->dma_mask;
> + mc_dev->dev.coherent_dma_mask = mc_dev->dma_mask;
>   dev_set_msi_domain(_dev->dev,
>  dev_get_msi_domain(_mc_dev->dev));
>   }
> @@ -633,10 +645,6 @@ int fsl_mc_device_add(struct fsl_mc_obj_desc *obj_desc,
>   goto error_cleanup_dev;
>   }
>
> - /* Objects are coherent, unless 'no shareability' flag set. */
> - if (!(obj_desc->flags & FSL_MC_OBJ_FLAG_NO_MEM_SHAREABILITY))
> - arch_setup_dma_ops(_dev->dev, 0, 0, NULL, true);
> -
>   /*
>* The device-specific probe callback will get invoked by device_add()
>*/
>

[PATCH v2] mmc: sdhci-of-esdhc: set proper dma mask for ls104x chips

[Laurentiu Tudor]

SDHCI controller in ls1043a and ls1046a generate 40-bit wide addresses
when doing DMA. Make sure that the corresponding dma mask is correctly
configured.

Context: when enabling smmu on these chips the following problem is
encountered: the smmu input address size is 48 bits so the dma mappings
for sdhci end up 48-bit wide. However, on these chips sdhci only use
40-bits of that address size when doing dma.
So you end up with a 48-bit address translation in smmu but the device
generates transactions with clipped 40-bit addresses, thus smmu context
faults are triggered. Setting up the correct dma mask fixes this
situation.

Signed-off-by: Laurentiu Tudor 
---
Changes in v2:
 - updated commit log with some context

 drivers/mmc/host/sdhci-of-esdhc.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/mmc/host/sdhci-of-esdhc.c 
b/drivers/mmc/host/sdhci-of-esdhc.c
index 4ffa6b173a21..8332f56e6c0d 100644
--- a/drivers/mmc/host/sdhci-of-esdhc.c
+++ b/drivers/mmc/host/sdhci-of-esdhc.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include "sdhci-pltfm.h"
 #include "sdhci-esdhc.h"
@@ -427,6 +428,11 @@ static void esdhc_of_adma_workaround(struct sdhci_host 
*host, u32 intmask)
 static int esdhc_of_enable_dma(struct sdhci_host *host)
 {
u32 value;
+   struct device *dev = mmc_dev(host->mmc);
+
+   if (of_device_is_compatible(dev->of_node, "fsl,ls1043a-esdhc") ||
+   of_device_is_compatible(dev->of_node, "fsl,ls1046a-esdhc"))
+   dma_set_mask_and_coherent(dev, DMA_BIT_MASK(40));
 
value = sdhci_readl(host, ESDHC_DMA_SYSCTL);
value |= ESDHC_DMA_SNOOP;
-- 
2.17.1

Re: [PATCH 1/2] staging:fsl-mc: Move DPIO from staging to drivers/soc/fsl

[Laurentiu Tudor]

Hi Roy,

Couple of comments inline.

On 05.07.2018 22:41, Roy Pledge wrote:
> Move the NXP DPIO (Datapath I/O Driver) out of the
> drivers/staging directory and into the drivers/soc/fsl directory.
> 
> The DPIO driver enables access to Queue and Buffer Manager (QBMAN)
> hardware on NXP DPAA2 devices. This is a prerequisite to moving the
> DPAA2 Ethernet driver out of staging.
> 
> Signed-off-by: Roy Pledge 
> ---
>   MAINTAINERS|  2 +-
>   drivers/crypto/caam/sg_sw_qm2.h|  2 +-
>   drivers/crypto/caam/sg_sw_sec4.h   |  2 +-
>   drivers/soc/fsl/Kconfig| 10 
> ++
>   drivers/soc/fsl/Makefile   |  1 +
>   drivers/{staging/fsl-mc/bus => soc/fsl}/dpio/Makefile  |  0
>   drivers/{staging/fsl-mc/bus => soc/fsl}/dpio/dpio-cmd.h|  0
>   drivers/{staging/fsl-mc/bus => soc/fsl}/dpio/dpio-driver.c |  2 +-
>   drivers/{staging/fsl-mc/bus => soc/fsl}/dpio/dpio-driver.txt   |  0

Maybe this should be converted to .rst and go in the already existing
Documentation/networking/dpaa2/?

>   drivers/{staging/fsl-mc/bus => soc/fsl}/dpio/dpio-service.c|  2 +-
>   drivers/{staging/fsl-mc/bus => soc/fsl}/dpio/dpio.c|  0
>   drivers/{staging/fsl-mc/bus => soc/fsl}/dpio/dpio.h|  0
>   drivers/{staging/fsl-mc/bus => soc/fsl}/dpio/qbman-portal.c|  2 +-
>   drivers/{staging/fsl-mc/bus => soc/fsl}/dpio/qbman-portal.h|  2 +-
>   drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h |  4 ++--
>   drivers/staging/fsl-mc/bus/Kconfig |  9 
> -
>   drivers/staging/fsl-mc/bus/Makefile|  2 --
>   {drivers/staging/fsl-mc/include => include/soc/fsl}/dpaa2-fd.h |  0
>   .../staging/fsl-mc/include => include/soc/fsl}/dpaa2-global.h  |  0
>   {drivers/staging/fsl-mc/include => include/soc/fsl}/dpaa2-io.h |  0
>   20 files changed, 20 insertions(+), 20 deletions(-)
>   rename drivers/{staging/fsl-mc/bus => soc/fsl}/dpio/Makefile (100%)
>   rename drivers/{staging/fsl-mc/bus => soc/fsl}/dpio/dpio-cmd.h (100%)
>   rename drivers/{staging/fsl-mc/bus => soc/fsl}/dpio/dpio-driver.c (99%)
>   rename drivers/{staging/fsl-mc/bus => soc/fsl}/dpio/dpio-driver.txt (100%)
>   rename drivers/{staging/fsl-mc/bus => soc/fsl}/dpio/dpio-service.c (99%)
>   rename drivers/{staging/fsl-mc/bus => soc/fsl}/dpio/dpio.c (100%)
>   rename drivers/{staging/fsl-mc/bus => soc/fsl}/dpio/dpio.h (100%)
>   rename drivers/{staging/fsl-mc/bus => soc/fsl}/dpio/qbman-portal.c (99%)
>   rename drivers/{staging/fsl-mc/bus => soc/fsl}/dpio/qbman-portal.h (99%)
>   rename {drivers/staging/fsl-mc/include => include/soc/fsl}/dpaa2-fd.h (100%)
>   rename {drivers/staging/fsl-mc/include => include/soc/fsl}/dpaa2-global.h 
> (100%)
>   rename {drivers/staging/fsl-mc/include => include/soc/fsl}/dpaa2-io.h (100%)

I received feedback in the past on mc-bus that a driver should limit to 
only one public header and one private one. Would it make sense to do 
the same for dpio too?

---
Best Regards, Laurentiu

Re: [PATCH v6 0/6] staging: Introduce DPAA2 Ethernet Switch driver

[Laurentiu Tudor]

Hi Dan,

On 03/15/2018 12:56 PM, Dan Carpenter wrote:
> On Thu, Mar 15, 2018 at 12:44:37AM +0100, Andrew Lunn wrote:
>> On Wed, Mar 14, 2018 at 10:55:52AM -0500, Razvan Stefanescu wrote:
>>> This patchset introduces the Ethernet Switch Driver for Freescale/NXP SoCs
>>> with DPAA2 (DataPath Acceleration Architecture v2). The driver manages
>>> switch objects discovered on the fsl-mc bus. A description of the driver
>>> can be found in the associated README file.
>>
>> Hi Greg
>>
>> This code has much better quality than the usual stuff in staging. I
>> see no reason not to merge it.
>
> Yeah.  It seems pretty decent.  Stuart, Laurentiu, care to comment?

Not sure on what you want us to comment ...

> Meanwhile, netdev and DaveM aren't even on the CC list and they're the
> ones to ultimately decide.

I think we'll post to netdev when we'll be done with the TODOs
and start moving the driver out of staging.

---
Best Regards, Laurentiu

Re: [PATCH v3 2/4] bus: fsl-mc: add restool userspace support

[Laurentiu Tudor]

Hello,

My 2c below.

On 04/04/2018 03:42 PM, Andrew Lunn wrote:
>> I hear you.  It is more complicated this way...having all these individual
>> objects vs just a single "bundle" of them that represents a NIC.  But, that's
>> the way the DPAA2 hardware is, and we're implementing kernel support for
>> the hardware as it is.
>
> Hi Stuart
>
> I see we are not making any progress here.
>
> So what i suggest is you post the kernel code and configuration tool
> concept to netdev for a full review. You want reviews from David
> Miller, Jiri Pirko, Jakub Kicinski, David Ahern, etc.
>

I think that the discussion steered too much towards networking related 
topics, while this ioctl doesn't have much to do with networking.
It's just an ioctl for our mc-bus bus driver that is used to manage the 
devices on this bus through userspace tools.
In addition, I'd drop any mention of our reference user space app 
(restool) to emphasize that this ioctl is not added just for a 
particular user space app. I think Stuart also mentioned this.

---
Thanks & Best Regards, Laurentiu

Re: [PATCH v3 2/4] bus: fsl-mc: add restool userspace support

[Laurentiu Tudor]

On 04/05/2018 02:47 PM, Andrew Lunn wrote:
> On Thu, Apr 05, 2018 at 10:30:01AM +0000, Laurentiu Tudor wrote:
>> Hello,
>>
>> My 2c below.
>>
>> On 04/04/2018 03:42 PM, Andrew Lunn wrote:
>>>> I hear you.  It is more complicated this way...having all these individual
>>>> objects vs just a single "bundle" of them that represents a NIC.  But, 
>>>> that's
>>>> the way the DPAA2 hardware is, and we're implementing kernel support for
>>>> the hardware as it is.
>>>
>>> Hi Stuart
>>>
>>> I see we are not making any progress here.
>>>
>>> So what i suggest is you post the kernel code and configuration tool
>>> concept to netdev for a full review. You want reviews from David
>>> Miller, Jiri Pirko, Jakub Kicinski, David Ahern, etc.
>>>
>>
>> I think that the discussion steered too much towards networking related
>> topics, while this ioctl doesn't have much to do with networking.
>
> Hi Laurentiu
>
> So i can use switchdev without it? I can modprobe the switchdev
> driver, all the physical interfaces will appear, and i can use ip addr
> add etc. I do not need to use a user space tool at all in order to use
> the network functionality?

Absolutely!
In normal use cases the system designer, depending on the requirements, 
configures the various devices that it desires through a firmware 
configuration (think something like a device tree). The devices 
configured are presented on the mc-bus and probed normally by the 
kernel. The standard networking linux tools can be used as expected.

The ioctl is necessary only for more advanced use cases that are 
supported by this bus. Think "more dynamic" scenarios that involve 
linking & unlinking various devices at runtime, maybe some 
virtualization scenarios. Unfortunately I'm not the architect type of 
guy so I don't have more specific examples to better illustrate ...

---
Best Regards, Laurentiu

Re: [PATCH v3 2/4] bus: fsl-mc: add restool userspace support

[Laurentiu Tudor]

Hi Greg,

On 04/05/2018 03:30 PM, gregkh wrote:
> On Thu, Apr 05, 2018 at 10:30:01AM +0000, Laurentiu Tudor wrote:
>> Hello,
>>
>> My 2c below.
>>
>> On 04/04/2018 03:42 PM, Andrew Lunn wrote:
>>>> I hear you.  It is more complicated this way...having all these individual
>>>> objects vs just a single "bundle" of them that represents a NIC.  But, 
>>>> that's
>>>> the way the DPAA2 hardware is, and we're implementing kernel support for
>>>> the hardware as it is.
>>>
>>> Hi Stuart
>>>
>>> I see we are not making any progress here.
>>>
>>> So what i suggest is you post the kernel code and configuration tool
>>> concept to netdev for a full review. You want reviews from David
>>> Miller, Jiri Pirko, Jakub Kicinski, David Ahern, etc.
>>>
>>
>> I think that the discussion steered too much towards networking related
>> topics, while this ioctl doesn't have much to do with networking.
>> It's just an ioctl for our mc-bus bus driver that is used to manage the
>> devices on this bus through userspace tools.
>> In addition, I'd drop any mention of our reference user space app
>> (restool) to emphasize that this ioctl is not added just for a
>> particular user space app. I think Stuart also mentioned this.
>
> I'm not going to take a "generic device configuration ioctl" patch
> unless it is documented to all exactly what it does, and why it is
> there.

The ioctl() is just a simple pass-through interface to the firmware.
It passes commands to the firmware and returns the response back to the 
userspace. Thus the ABI used by the firmware applies for this ioctl() 
and it is documented in detail here:

https://www.nxp.com/docs/en/user-guide/DPAA2_UM.pdf

---
Best Regards, Laurentiu

Re: [PATCH v3 2/4] bus: fsl-mc: add restool userspace support

[Laurentiu Tudor]

Hi Andrew,

On 04/05/2018 03:48 PM, Andrew Lunn wrote:
>>> Hi Laurentiu
>>>
>>> So i can use switchdev without it? I can modprobe the switchdev
>>> driver, all the physical interfaces will appear, and i can use ip addr
>>> add etc. I do not need to use a user space tool at all in order to use
>>> the network functionality?
>>
>> Absolutely!
>
> Great.
>
> Then the easiest way forwards is to simply drop the IOCTL code for the
> moment. Get the basic support for the hardware into the kernel
> first. Then come back later to look at dynamic behaviour which needs
> some form of configuration.

Hmm, not sure I understand. We already have a fully functional ethernet 
driver [1] and a switch driver [2] ...

>> In normal use cases the system designer, depending on the requirements,
>> configures the various devices that it desires through a firmware
>> configuration (think something like a device tree). The devices
>> configured are presented on the mc-bus and probed normally by the
>> kernel. The standard networking linux tools can be used as expected.
>
> So what you should probably do is start a discussion on what this
> device tree binding looks like. But you need to be careful even
> here. Device tree describes the hardware, not how you configure the
> hardware. So maybe DT does not actually fit.

It's not an actual device tree, but a configuration file that happens to 
reuse the DTS format. I guess my analogy with a device tree was not the 
best.
Detailed documentation on the syntax can be found here [3], chapter 22.

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/staging/fsl-dpaa2/ethernet
[2] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/staging/fsl-dpaa2/ethsw
[3] https://www.nxp.com/docs/en/user-guide/DPAA2_UM.pdf

---
Best Regards, Laurentiu

Re: [PATCH 2/3] staging: fsl-mc: Move DPBP out of staging

[Laurentiu Tudor]

Hi Bogdan,

On 03/01/2018 07:47 PM, Bogdan Purcareata wrote:
> Move the source files out of staging into their final locations:
> - dpbp.c goes to drivers/bus/fsl-mc/, next to the core infrastructure
> - dpbp-cmd.h gets merged into drivers/bus/fsl-mc/fsl-mc-private.h, next
>to the other internally used APIs
> - dpbp.h gets merged into include/linux/fsl/mc.h, exposing the public
>API
>
> Update references in the dpaa2-eth staging driver.
>
> DPBP stands for Data Path Buffer Pool - you can read more about the
> object in Documentation/networking/dpaa2/overview.rst
>
> Signed-off-by: Bogdan Purcareata <bogdan.purcare...@nxp.com>
> ---
>   drivers/bus/fsl-mc/Makefile   |  1 +
>   drivers/{staging/fsl-mc/bus => bus/fsl-mc}/dpbp.c |  4 +-
>   drivers/bus/fsl-mc/fsl-mc-private.h   | 39 +
>   drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h|  2 +-
>   drivers/staging/fsl-mc/bus/Makefile   |  3 +-
>   drivers/staging/fsl-mc/bus/dpbp-cmd.h | 44 ---
>   drivers/staging/fsl-mc/include/dpbp.h | 53 
> ---
>   include/linux/fsl/mc.h| 42 ++
>   8 files changed, 86 insertions(+), 102 deletions(-)
>   rename drivers/{staging/fsl-mc/bus => bus/fsl-mc}/dpbp.c (98%)
>   delete mode 100644 drivers/staging/fsl-mc/bus/dpbp-cmd.h
>   delete mode 100644 drivers/staging/fsl-mc/include/dpbp.h
>
> diff --git a/drivers/bus/fsl-mc/Makefile b/drivers/bus/fsl-mc/Makefile
> index 6a97f2c..da26e52 100644
> --- a/drivers/bus/fsl-mc/Makefile
> +++ b/drivers/bus/fsl-mc/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_FSL_MC_BUS) += mc-bus-driver.o
>   mc-bus-driver-objs := fsl-mc-bus.o \
> mc-sys.o \
> mc-io.o \
> +   dpbp.o \
> dprc.o \
> dprc-driver.o \
> fsl-mc-allocator.o \
> diff --git a/drivers/staging/fsl-mc/bus/dpbp.c b/drivers/bus/fsl-mc/dpbp.c
> similarity index 98%
> rename from drivers/staging/fsl-mc/bus/dpbp.c
> rename to drivers/bus/fsl-mc/dpbp.c
> index 85735bb..31a360b 100644
> --- a/drivers/staging/fsl-mc/bus/dpbp.c
> +++ b/drivers/bus/fsl-mc/dpbp.c
> @@ -5,9 +5,9 @@
>*/
>   #include 
>   #include 
> -#include "../include/dpbp.h"
> +#include "linux/fsl/mc.h"

I think we can use <> here, same comment for patch 3/3.

Other than that, the series looks ok to me so for all the patches:

Reviewed-by: Laurentiu Tudor <laurentiu.tu...@nxp.com>

---
Best Regards, Laurentiu

> -#include "dpbp-cmd.h"
> +#include "fsl-mc-private.h"
>
>   /**
>* dpbp_open() - Open a control session for the specified object.
> diff --git a/drivers/bus/fsl-mc/fsl-mc-private.h 
> b/drivers/bus/fsl-mc/fsl-mc-private.h
> index bed990c..4ef8d7e 100644
> --- a/drivers/bus/fsl-mc/fsl-mc-private.h
> +++ b/drivers/bus/fsl-mc/fsl-mc-private.h
> @@ -379,6 +379,45 @@ int dprc_get_container_id(struct fsl_mc_io *mc_io,
> u32 cmd_flags,
> int *container_id);
>
> +/*
> + * Data Path Buffer Pool (DPBP) API
> + */
> +
> +/* DPBP Version */
> +#define DPBP_VER_MAJOR   3
> +#define DPBP_VER_MINOR   2
> +
> +/* Command versioning */
> +#define DPBP_CMD_BASE_VERSION1
> +#define DPBP_CMD_ID_OFFSET   4
> +
> +#define DPBP_CMD(id) (((id) << DPBP_CMD_ID_OFFSET) | DPBP_CMD_BASE_VERSION)
> +
> +/* Command IDs */
> +#define DPBP_CMDID_CLOSE DPBP_CMD(0x800)
> +#define DPBP_CMDID_OPEN  DPBP_CMD(0x804)
> +
> +#define DPBP_CMDID_ENABLEDPBP_CMD(0x002)
> +#define DPBP_CMDID_DISABLE   DPBP_CMD(0x003)
> +#define DPBP_CMDID_GET_ATTR  DPBP_CMD(0x004)
> +#define DPBP_CMDID_RESET DPBP_CMD(0x005)
> +
> +struct dpbp_cmd_open {
> + __le32 dpbp_id;
> +};
> +
> +#define DPBP_ENABLE  0x1
> +
> +struct dpbp_rsp_get_attributes {
> + /* response word 0 */
> + __le16 pad;
> + __le16 bpid;
> + __le32 id;
> + /* response word 1 */
> + __le16 version_major;
> + __le16 version_minor;
> +};
> +
>   /**
>* Maximum number of total IRQs that can be pre-allocated for an MC bus'
>* IRQ pool
> diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h 
> b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
> index e577410..ce864ee 100644
> --- a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
> +++ b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
> @

[PATCH] mmc: sdhci-of-esdhc: set proper dma mask for ls104x chips

[Laurentiu Tudor]

SDHCI controller in ls1043a and ls1046a generate 40-bit wide addresses
when doing DMA. Make sure that the corresponding dma mask is correctly
configured.

Signed-off-by: Laurentiu Tudor 
---
 drivers/mmc/host/sdhci-of-esdhc.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/mmc/host/sdhci-of-esdhc.c 
b/drivers/mmc/host/sdhci-of-esdhc.c
index 4ffa6b173a21..8332f56e6c0d 100644
--- a/drivers/mmc/host/sdhci-of-esdhc.c
+++ b/drivers/mmc/host/sdhci-of-esdhc.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include "sdhci-pltfm.h"
 #include "sdhci-esdhc.h"
@@ -427,6 +428,11 @@ static void esdhc_of_adma_workaround(struct sdhci_host 
*host, u32 intmask)
 static int esdhc_of_enable_dma(struct sdhci_host *host)
 {
u32 value;
+   struct device *dev = mmc_dev(host->mmc);
+
+   if (of_device_is_compatible(dev->of_node, "fsl,ls1043a-esdhc") ||
+   of_device_is_compatible(dev->of_node, "fsl,ls1046a-esdhc"))
+   dma_set_mask_and_coherent(dev, DMA_BIT_MASK(40));
 
value = sdhci_readl(host, ESDHC_DMA_SYSCTL);
value |= ESDHC_DMA_SNOOP;
-- 
2.17.1

Re: [PATCH v1 1/2] bus: mc-bus: Add support for mapping shareable portals

[Laurentiu Tudor]

Hi Roy,
On 30.10.2018 22:30, Roy Pledge wrote:
> Starting with v5 of NXP QBMan devices the hardware supports using
> regular cacheable/shareable memory as the backing store for the
> portals.
> 
> This patch adds support for the new portal mode by switching to
> use the DPRC get object region v2 command which returns both
> a base address and offset for the portal memory. The new portal
> region is identified as shareable through the addition of a new
> flag.
> 
> Signed-off-by: Roy Pledge 
> ---
>   drivers/bus/fsl-mc/dprc.c   |  3 ++-
>   drivers/bus/fsl-mc/fsl-mc-bus.c | 14 --
>   drivers/bus/fsl-mc/fsl-mc-private.h | 17 ++---
>   3 files changed, 28 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/bus/fsl-mc/dprc.c b/drivers/bus/fsl-mc/dprc.c
> index 1c3f621..bde856d 100644
> --- a/drivers/bus/fsl-mc/dprc.c
> +++ b/drivers/bus/fsl-mc/dprc.c
> @@ -461,8 +461,9 @@ int dprc_get_obj_region(struct fsl_mc_io *mc_io,
>   
>   /* retrieve response parameters */
>   rsp_params = (struct dprc_rsp_get_obj_region *)cmd.params;
> - region_desc->base_offset = le64_to_cpu(rsp_params->base_addr);
> + region_desc->base_offset = le64_to_cpu(rsp_params->base_offset);
>   region_desc->size = le32_to_cpu(rsp_params->size);
> + region_desc->base_address = le64_to_cpu(rsp_params->base_addr);
>   
>   return 0;
>   }
> diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c
> index f0404c6..25ad422 100644
> --- a/drivers/bus/fsl-mc/fsl-mc-bus.c
> +++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
> @@ -487,10 +487,18 @@ static int fsl_mc_device_get_mmio_regions(struct 
> fsl_mc_device *mc_dev,
>   "dprc_get_obj_region() failed: %d\n", error);
>   goto error_cleanup_regions;
>   }
> -
> - error = translate_mc_addr(mc_dev, mc_region_type,
> + /* Older MC only returned region offset and no base address
> +  * If base address is in the region_desc use it otherwise
> +  * revert to old mechanism
> +  */
> + if (region_desc.base_address)
> + regions[i].start = region_desc.base_address +
> + region_desc.base_offset;
> + else
> + error = translate_mc_addr(mc_dev, mc_region_type,
> region_desc.base_offset,
> [i].start);
> +
>   if (error < 0) {
>   dev_err(parent_dev,
>   "Invalid MC offset: %#x (for %s.%d\'s region 
> %d)\n",
> @@ -504,6 +512,8 @@ static int fsl_mc_device_get_mmio_regions(struct 
> fsl_mc_device *mc_dev,
>   regions[i].flags = IORESOURCE_IO;
>   if (region_desc.flags & DPRC_REGION_CACHEABLE)
>   regions[i].flags |= IORESOURCE_CACHEABLE;
> + if (region_desc.flags & DPRC_REGION_SHAREABLE)
> + regions[i].flags |= IORESOURCE_MEM;
>   }
>   
>   mc_dev->regions = regions;
> diff --git a/drivers/bus/fsl-mc/fsl-mc-private.h 
> b/drivers/bus/fsl-mc/fsl-mc-private.h
> index ea11b4f..28e40d1 100644
> --- a/drivers/bus/fsl-mc/fsl-mc-private.h
> +++ b/drivers/bus/fsl-mc/fsl-mc-private.h
> @@ -79,9 +79,11 @@ int dpmcp_reset(struct fsl_mc_io *mc_io,
>   
>   /* DPRC command versioning */
>   #define DPRC_CMD_BASE_VERSION   1
> +#define DPRC_CMD_2ND_VERSION 2
>   #define DPRC_CMD_ID_OFFSET  4
>   
>   #define DPRC_CMD(id)(((id) << DPRC_CMD_ID_OFFSET) | 
> DPRC_CMD_BASE_VERSION)
> +#define DPRC_CMD_V2(id)  (((id) << DPRC_CMD_ID_OFFSET) | 
> DPRC_CMD_2ND_VERSION)
>   
>   /* DPRC command IDs */
>   #define DPRC_CMDID_CLOSEDPRC_CMD(0x800)
> @@ -99,7 +101,7 @@ int dpmcp_reset(struct fsl_mc_io *mc_io,
>   #define DPRC_CMDID_GET_CONT_ID  DPRC_CMD(0x830)
>   #define DPRC_CMDID_GET_OBJ_COUNTDPRC_CMD(0x159)
>   #define DPRC_CMDID_GET_OBJ  DPRC_CMD(0x15A)
> -#define DPRC_CMDID_GET_OBJ_REG  DPRC_CMD(0x15E)
> +#define DPRC_CMDID_GET_OBJ_REG  DPRC_CMD_V2(0x15E)

I see you're bumping this command's version to v2. Will we still be 
compatible with older MC firmware versions?

---
Best Regards, Laurentiu

Re: [PATCH] bus: fsl-mc: explicitly define the fsl_mc_command endianness

[Laurentiu Tudor]

On 02.10.2018 15:16, Ioana Ciornei wrote:
> Both the header and the command parameters of the fsl_mc_command are
> 64-bit little-endian words. Use the appropriate type to explicitly
> specify their endianness.
> 
> Signed-off-by: Ioana Ciornei 

Reviewed-By: Laurentiu Tudor 

---
Best Regards, Laurentiu

> ---
>   include/linux/fsl/mc.h | 12 ++--
>   1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/fsl/mc.h b/include/linux/fsl/mc.h
> index f27cb14..96c54bb 100644
> --- a/include/linux/fsl/mc.h
> +++ b/include/linux/fsl/mc.h
> @@ -210,8 +210,8 @@ struct mc_cmd_header {
>   };
>   
>   struct fsl_mc_command {
> - u64 header;
> - u64 params[MC_CMD_NUM_OF_PARAMS];
> + __le64 header;
> + __le64 params[MC_CMD_NUM_OF_PARAMS];
>   };
>   
>   enum mc_cmd_status {
> @@ -238,11 +238,11 @@ enum mc_cmd_status {
>   /* Command completion flag */
>   #define MC_CMD_FLAG_INTR_DIS0x01
>   
> -static inline u64 mc_encode_cmd_header(u16 cmd_id,
> -u32 cmd_flags,
> -u16 token)
> +static inline __le64 mc_encode_cmd_header(u16 cmd_id,
> +   u32 cmd_flags,
> +   u16 token)
>   {
> - u64 header = 0;
> + __le64 header = 0;
>   struct mc_cmd_header *hdr = (struct mc_cmd_header *)
>   
>   hdr->cmd_id = cpu_to_le16(cmd_id);
> 

[PATCH v3 16/22] arm64: dts: ls1046a: add smmu node

[laurentiu . tudor]

From: Laurentiu Tudor 

This allows for the SMMU device to be probed by the SMMU kernel driver.

Signed-off-by: Laurentiu Tudor 
---
 .../arm64/boot/dts/freescale/fsl-ls1046a.dtsi | 42 +++
 1 file changed, 42 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi 
b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
index ef83786b8b90..07a853a0aeaa 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
@@ -228,6 +228,48 @@
bus-width = <4>;
};
 
+   mmu: iommu@900 {
+   compatible = "arm,mmu-500";
+   reg = <0 0x900 0 0x40>;
+   dma-coherent;
+   #global-interrupts = <2>;
+   #iommu-cells = <1>;
+   interrupts = ,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+;
+   };
+
scfg: scfg@157 {
compatible = "fsl,ls1046a-scfg", "syscon";
reg = <0x0 0x157 0x0 0x1>;
-- 
2.17.1

Re: [GIT PULL] fixes for soc/fsl drivers for v4.19 take 2

[Laurentiu Tudor]

Hi Leo,

On 02.10.2018 01:58, Li Yang wrote:
> Hi arm-soc maintainers,
> 
> Please merge the updated fix for the following issue.
> 
> Regards,
> Leo
> 
> The following changes since commit 96fc74333f84cfdf8d434c6c07254e215e2aad00:
> 
>soc: fsl: qe: Fix copy/paste bug in ucc_get_tdm_sync_shift() (2018-09-25 
> 13:57:26 -0700)
> 
> are available in the Git repository at:
> 
>git://git.kernel.org/pub/scm/linux/kernel/git/leo/linux.git 
> soc-fsl-fix-v4.19-2
> 
> for you to fetch changes up to 5a1eb8b9542884592a018829bb1ff20c9695d925:
> 
>soc: fsl: qman_portals: defer probe after qman's probe (2018-10-01 
> 17:47:43 -0500)
> 
> 
> NXP/FSL SoC driver fixes for v4.19 round 2
> 
> - Fix crash of qman_portal by deferring its probe if qman is not probed
> 
> 
> Laurentiu Tudor (2):
>soc: fsl: qbman: add APIs to retrieve the probing status
>soc: fsl: qman_portals: defer probe after qman's probe

There's a similar fix for bman portals [1]. I was under the impression 
that you plan to pick that up too.

[1] "soc/fsl/bman_portals: defer probe after bman's probe", found here:
https://lore.kernel.org/patchwork/patch/992009/

---
Thanks & Best Regards, Laurentiu

Re: [PATCH 00/21] SMMU enablement for NXP LS1043A and LS1046A

[Laurentiu Tudor]

Hi Robin,

On 19.09.2018 16:25, Robin Murphy wrote:
> Hi Laurentiu,
> 
> On 19/09/18 13:35, laurentiu.tu...@nxp.com wrote:
>> From: Laurentiu Tudor 
>>
>> This patch series adds SMMU support for NXP LS1043A and LS1046A chips
>> and consists mostly in important driver fixes and the required device
>> tree updates. It touches several subsystems and consists of three main
>> parts:
>>   - changes in soc/drivers/fsl/qbman drivers adding iommu mapping of
>>     reserved memory areas, fixes and defered probe support
>>   - changes in drivers/net/ethernet/freescale/dpaa_eth drivers
>>     consisting in misc dma mapping related fixes and probe ordering
>>   - addition of the actual arm smmu device tree node together with
>>     various adjustments to the device trees
>>
>> Performance impact
>>
>>  Running iperf benchmarks in a back-to-back setup (both sides
>>  having smmu enabled) on a 10GBps port show an important
>>  networking performance degradation of around %40 (9.48Gbps
>>  linerate vs 5.45Gbps). If you need performance but without
>>  SMMU support you can use "iommu.passthrough=1" to disable
>>  SMMU.
>>
>> USB issue and workaround
>>
>>  There's a problem with the usb controllers in these chips
>>  generating smaller, 40-bit wide dma addresses instead of the 48-bit
>>  supported at the smmu input. So you end up in a situation where the
>>  smmu is mapped with 48-bit address translations, but the device
>>  generates transactions with clipped 40-bit addresses, thus smmu
>>  context faults are triggered. I encountered a similar situation for
>>  mmc that I  managed to fix in software [1] however for USB I did not
>>  find a proper place in the code to add a similar fix. The only
>>  workaround I found was to add this kernel parameter which limits the
>>  usb dma to 32-bit size: "xhci-hcd.quirks=0x80".
>>  This workaround if far from ideal, so any suggestions for a code
>>  based workaround in this area would be greatly appreciated.
> 
> If you have a nominally-64-bit device with a 
> narrower-than-the-main-interconnect link in front of it, that should 
> already be fixed in 4.19-rc by bus_dma_mask picking up DT dma-ranges, 
> provided the interconnect hierarchy can be described appropriately (or 
> at least massaged sufficiently to satisfy the binding), e.g.:
> 
> / {
>  ...
> 
>  soc {
>      ranges;
>      dma-ranges = <0 0 1 0>;
> 
>      dev_48bit { ... };
> 
>      periph_bus {
>      ranges;
>      dma-ranges = <0 0 100 0>;
> 
>      dev_40bit { ... };
>      };
>  };
> };
> 
> and if that fails to work as expected (except for PCI hosts where 
> handling dma-ranges properly still needs sorting out), please do let us 
> know ;)
> 

Just to confirm, Is this [1] the change I was supposed to test?
Because if so, I'm still seeing context faults [2] with what looks like 
clipped to 40-bits addresses. :-(
IIRC, the usb subsystem explicitly set 64-bit dma masks which in turn 
will be limited to the SMMU input size of 48-bit. Won't that overwrite 
the default dma mask derived from dma-ranges?

---
Best Regards, Laurentiu

[1] -

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi 
b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
index 3bdea0470f69..a214c3df37fd 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
@@ -612,6 +612,7 @@
 compatible = "snps,dwc3";
 reg = <0x0 0x2f0 0x0 0x1>;
 interrupts = ;
+   dma-ranges = <0x0 0x0 0x0 0x0 0x100 0x>;
 dr_mode = "host";
 snps,quirk-frame-length-adjustment = <0x20>;
 snps,dis_rxdet_inp3_quirk;
@@ -621,6 +622,7 @@
 compatible = "snps,dwc3";
 reg = <0x0 0x300 0x0 0x1>;
 interrupts = ;
+   dma-ranges = <0x0 0x0 0x0 0x0 0x100 0x>;
 dr_mode = "host";
 snps,quirk-frame-length-adjustment = <0x20>;
 snps,dis_rxdet_inp3_quirk;
@@ -630,6 +632,7 @@
 compatible = "snps,dwc3";
 reg = <0x0 0x310 0x0 0x1>;
 interrupts = ;
+ 

[PATCH v2 18/22] arm64: dts: ls104xa: set mask to drop TBU ID from StreamID

[laurentiu . tudor]

From: Laurentiu Tudor 

The StreamID entering the SMMU is actually a concatenation of the
SMMU TBU ID and the ICID configured in software.
Since the TBU ID is internal to the SoC and since we want that the
actual the ICID configured in software to enter the SMMU witout any
additional set bits, mask out the TBU ID bits and leave only the
relevant ICID bits to enter SMMU.

Signed-off-by: Laurentiu Tudor 
---
 arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 1 +
 arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi | 1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi 
b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
index 7eea2bace171..1f9b385007a8 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
@@ -226,6 +226,7 @@
compatible = "arm,mmu-500";
reg = <0 0x900 0 0x40>;
dma-coherent;
+   stream-match-mask = <0x7f00>;
#global-interrupts = <2>;
#iommu-cells = <1>;
interrupts = ,
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi 
b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
index 07a853a0aeaa..22bf3975492a 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
@@ -232,6 +232,7 @@
compatible = "arm,mmu-500";
reg = <0 0x900 0 0x40>;
dma-coherent;
+   stream-match-mask = <0x7f00>;
#global-interrupts = <2>;
#iommu-cells = <1>;
interrupts = ,
-- 
2.17.1

[PATCH v2 15/22] dpaa_eth: fix SG frame cleanup

[laurentiu . tudor]

From: Laurentiu Tudor 

Fix issue with the entry indexing in the sg frame cleanup code being
off-by-1. This problem showed up when doing some basic iperf tests and
manifested in traffic coming to a halt.

Signed-off-by: Laurentiu Tudor 
---
 drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c 
b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
index 8db861f281a0..605f06f0def8 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
@@ -1663,7 +1663,7 @@ static struct sk_buff *dpaa_cleanup_tx_fd(const struct 
dpaa_priv *priv,
 qm_sg_entry_get_len([0]), dma_dir);
 
/* remaining pages were mapped with skb_frag_dma_map() */
-   for (i = 1; i < nr_frags; i++) {
+   for (i = 1; i <= nr_frags; i++) {
WARN_ON(qm_sg_entry_is_ext([i]));
 
dma_unmap_page(dev, qm_sg_addr([i]),
-- 
2.17.1

[PATCH v2 08/22] soc/fsl/qbman_portals: add APIs to retrieve the probing status

[laurentiu . tudor]

From: Laurentiu Tudor 

Add a couple of new APIs to check the probing status of the required
cpu bound qman and bman portals:
 'int bman_portals_probed()' and 'int qman_portals_probed()'.
They return the following values.
 *  1 if qman/bman portals were all probed correctly
 *  0 if qman/bman portals were not yet probed
 * -1 if probing of qman/bman portals failed
Drivers that use qman/bman portal driver services are required to use
these APIs before calling any functions exported by these drivers or
otherwise they will crash the kernel.
First user will be the dpaa1 ethernet driver, coming in a subsequent
patch.

Signed-off-by: Laurentiu Tudor 
---
 drivers/soc/fsl/qbman/bman_portal.c | 10 ++
 drivers/soc/fsl/qbman/qman_portal.c | 10 ++
 include/soc/fsl/bman.h  |  8 
 include/soc/fsl/qman.h  |  9 +
 4 files changed, 37 insertions(+)

diff --git a/drivers/soc/fsl/qbman/bman_portal.c 
b/drivers/soc/fsl/qbman/bman_portal.c
index f9edd28894fd..8048d35de8a2 100644
--- a/drivers/soc/fsl/qbman/bman_portal.c
+++ b/drivers/soc/fsl/qbman/bman_portal.c
@@ -32,6 +32,7 @@
 
 static struct bman_portal *affine_bportals[NR_CPUS];
 static struct cpumask portal_cpus;
+static int __bman_portals_probed;
 /* protect bman global registers and global data shared among portals */
 static DEFINE_SPINLOCK(bman_lock);
 
@@ -85,6 +86,12 @@ static int bman_online_cpu(unsigned int cpu)
return 0;
 }
 
+int bman_portals_probed(void)
+{
+   return __bman_portals_probed;
+}
+EXPORT_SYMBOL_GPL(bman_portals_probed);
+
 static int bman_portal_probe(struct platform_device *pdev)
 {
struct device *dev = >dev;
@@ -148,6 +155,7 @@ static int bman_portal_probe(struct platform_device *pdev)
spin_lock(_lock);
cpu = cpumask_next_zero(-1, _cpus);
if (cpu >= nr_cpu_ids) {
+   __bman_portals_probed = 1;
/* unassigned portal, skip init */
spin_unlock(_lock);
return 0;
@@ -173,6 +181,8 @@ static int bman_portal_probe(struct platform_device *pdev)
 err_ioremap2:
memunmap(pcfg->addr_virt_ce);
 err_ioremap1:
+__bman_portals_probed = 1;
+
return -ENXIO;
 }
 
diff --git a/drivers/soc/fsl/qbman/qman_portal.c 
b/drivers/soc/fsl/qbman/qman_portal.c
index eef93cab84f1..1b2fc981c269 100644
--- a/drivers/soc/fsl/qbman/qman_portal.c
+++ b/drivers/soc/fsl/qbman/qman_portal.c
@@ -39,6 +39,7 @@ EXPORT_SYMBOL(qman_dma_portal);
 #define CONFIG_FSL_DPA_PIRQ_FAST  1
 
 static struct cpumask portal_cpus;
+static int __qman_portals_probed;
 /* protect qman global registers and global data shared among portals */
 static DEFINE_SPINLOCK(qman_lock);
 
@@ -219,6 +220,12 @@ static int qman_online_cpu(unsigned int cpu)
return 0;
 }
 
+int qman_portals_probed(void)
+{
+   return __qman_portals_probed;
+}
+EXPORT_SYMBOL_GPL(qman_portals_probed);
+
 static int qman_portal_probe(struct platform_device *pdev)
 {
struct device *dev = >dev;
@@ -306,6 +313,7 @@ static int qman_portal_probe(struct platform_device *pdev)
spin_lock(_lock);
cpu = cpumask_next_zero(-1, _cpus);
if (cpu >= nr_cpu_ids) {
+   __qman_portals_probed = 1;
/* unassigned portal, skip init */
spin_unlock(_lock);
return 0;
@@ -336,6 +344,8 @@ static int qman_portal_probe(struct platform_device *pdev)
 err_ioremap2:
memunmap(pcfg->addr_virt_ce);
 err_ioremap1:
+   __qman_portals_probed = -1;
+
return -ENXIO;
 }
 
diff --git a/include/soc/fsl/bman.h b/include/soc/fsl/bman.h
index 5b99cb2ea5ef..173e4049d963 100644
--- a/include/soc/fsl/bman.h
+++ b/include/soc/fsl/bman.h
@@ -133,5 +133,13 @@ int bman_acquire(struct bman_pool *pool, struct bm_buffer 
*bufs, u8 num);
  * failed to probe or 0 if the bman driver did not probed yet.
  */
 int bman_is_probed(void);
+/**
+ * bman_portals_probed - Check if all cpu bound bman portals are probed
+ *
+ * Returns 1 if all the required cpu bound bman portals successfully probed,
+ * -1 if probe errors appeared or 0 if the bman portals did not yet finished
+ * probing.
+ */
+int bman_portals_probed(void);
 
 #endif /* __FSL_BMAN_H */
diff --git a/include/soc/fsl/qman.h b/include/soc/fsl/qman.h
index 597783b8a3a0..7732e48081eb 100644
--- a/include/soc/fsl/qman.h
+++ b/include/soc/fsl/qman.h
@@ -1194,4 +1194,13 @@ int qman_release_cgrid(u32 id);
  */
 int qman_is_probed(void);
 
+/**
+ * qman_portals_probed - Check if all cpu bound qman portals are probed
+ *
+ * Returns 1 if all the required cpu bound qman portals successfully probed,
+ * -1 if probe errors appeared or 0 if the qman portals did not yet finished
+ * probing.
+ */
+int qman_portals_probed(void);
+
 #endif /* __FSL_QMAN_H */
-- 
2.17.1

[PATCH v2 11/22] dpaa_eth: defer probing after qbman

[laurentiu . tudor]

From: Laurentiu Tudor 

Enabling SMMU altered the order of device probing causing the dpaa1
ethernet driver to get probed before qbman and causing a boot crash.
Add predictability in the probing order by deferring the ethernet
driver probe after qbman and portals by using the recently introduced
qbman APIs.

Signed-off-by: Laurentiu Tudor 
---
 .../net/ethernet/freescale/dpaa/dpaa_eth.c| 31 +++
 1 file changed, 31 insertions(+)

diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c 
b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
index a5131a510e8b..6ca3fdbef580 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
@@ -2765,6 +2765,37 @@ static int dpaa_eth_probe(struct platform_device *pdev)
int err = 0, i, channel;
struct device *dev;
 
+   err = bman_is_probed();
+   if (!err)
+   return -EPROBE_DEFER;
+   if (err < 0) {
+   dev_err(>dev, "failing probe due to bman probe error\n");
+   return -ENODEV;
+   }
+   err = qman_is_probed();
+   if (!err)
+   return -EPROBE_DEFER;
+   if (err < 0) {
+   dev_err(>dev, "failing probe due to qman probe error\n");
+   return -ENODEV;
+   }
+   err = bman_portals_probed();
+   if (!err)
+   return -EPROBE_DEFER;
+   if (err < 0) {
+   dev_err(>dev,
+   "failing probe due to bman portals probe error\n");
+   return -ENODEV;
+   }
+   err = qman_portals_probed();
+   if (!err)
+   return -EPROBE_DEFER;
+   if (err < 0) {
+   dev_err(>dev,
+   "failing probe due to qman portals probe error\n");
+   return -ENODEV;
+   }
+
/* device used for DMA mapping */
dev = pdev->dev.parent;
err = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(40));
-- 
2.17.1

Re: [PATCH 2/3] staging: fsl-mc: Move DPBP out of staging

[Laurentiu Tudor]

Hi Bogdan,

On 03/01/2018 07:47 PM, Bogdan Purcareata wrote:
> Move the source files out of staging into their final locations:
> - dpbp.c goes to drivers/bus/fsl-mc/, next to the core infrastructure
> - dpbp-cmd.h gets merged into drivers/bus/fsl-mc/fsl-mc-private.h, next
>to the other internally used APIs
> - dpbp.h gets merged into include/linux/fsl/mc.h, exposing the public
>API
>
> Update references in the dpaa2-eth staging driver.
>
> DPBP stands for Data Path Buffer Pool - you can read more about the
> object in Documentation/networking/dpaa2/overview.rst
>
> Signed-off-by: Bogdan Purcareata 
> ---
>   drivers/bus/fsl-mc/Makefile   |  1 +
>   drivers/{staging/fsl-mc/bus => bus/fsl-mc}/dpbp.c |  4 +-
>   drivers/bus/fsl-mc/fsl-mc-private.h   | 39 +
>   drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h|  2 +-
>   drivers/staging/fsl-mc/bus/Makefile   |  3 +-
>   drivers/staging/fsl-mc/bus/dpbp-cmd.h | 44 ---
>   drivers/staging/fsl-mc/include/dpbp.h | 53 
> ---
>   include/linux/fsl/mc.h| 42 ++
>   8 files changed, 86 insertions(+), 102 deletions(-)
>   rename drivers/{staging/fsl-mc/bus => bus/fsl-mc}/dpbp.c (98%)
>   delete mode 100644 drivers/staging/fsl-mc/bus/dpbp-cmd.h
>   delete mode 100644 drivers/staging/fsl-mc/include/dpbp.h
>
> diff --git a/drivers/bus/fsl-mc/Makefile b/drivers/bus/fsl-mc/Makefile
> index 6a97f2c..da26e52 100644
> --- a/drivers/bus/fsl-mc/Makefile
> +++ b/drivers/bus/fsl-mc/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_FSL_MC_BUS) += mc-bus-driver.o
>   mc-bus-driver-objs := fsl-mc-bus.o \
> mc-sys.o \
> mc-io.o \
> +   dpbp.o \
> dprc.o \
> dprc-driver.o \
> fsl-mc-allocator.o \
> diff --git a/drivers/staging/fsl-mc/bus/dpbp.c b/drivers/bus/fsl-mc/dpbp.c
> similarity index 98%
> rename from drivers/staging/fsl-mc/bus/dpbp.c
> rename to drivers/bus/fsl-mc/dpbp.c
> index 85735bb..31a360b 100644
> --- a/drivers/staging/fsl-mc/bus/dpbp.c
> +++ b/drivers/bus/fsl-mc/dpbp.c
> @@ -5,9 +5,9 @@
>*/
>   #include 
>   #include 
> -#include "../include/dpbp.h"
> +#include "linux/fsl/mc.h"

I think we can use <> here, same comment for patch 3/3.

Other than that, the series looks ok to me so for all the patches:

Reviewed-by: Laurentiu Tudor 

---
Best Regards, Laurentiu

> -#include "dpbp-cmd.h"
> +#include "fsl-mc-private.h"
>
>   /**
>* dpbp_open() - Open a control session for the specified object.
> diff --git a/drivers/bus/fsl-mc/fsl-mc-private.h 
> b/drivers/bus/fsl-mc/fsl-mc-private.h
> index bed990c..4ef8d7e 100644
> --- a/drivers/bus/fsl-mc/fsl-mc-private.h
> +++ b/drivers/bus/fsl-mc/fsl-mc-private.h
> @@ -379,6 +379,45 @@ int dprc_get_container_id(struct fsl_mc_io *mc_io,
> u32 cmd_flags,
> int *container_id);
>
> +/*
> + * Data Path Buffer Pool (DPBP) API
> + */
> +
> +/* DPBP Version */
> +#define DPBP_VER_MAJOR   3
> +#define DPBP_VER_MINOR   2
> +
> +/* Command versioning */
> +#define DPBP_CMD_BASE_VERSION1
> +#define DPBP_CMD_ID_OFFSET   4
> +
> +#define DPBP_CMD(id) (((id) << DPBP_CMD_ID_OFFSET) | DPBP_CMD_BASE_VERSION)
> +
> +/* Command IDs */
> +#define DPBP_CMDID_CLOSE DPBP_CMD(0x800)
> +#define DPBP_CMDID_OPEN  DPBP_CMD(0x804)
> +
> +#define DPBP_CMDID_ENABLEDPBP_CMD(0x002)
> +#define DPBP_CMDID_DISABLE   DPBP_CMD(0x003)
> +#define DPBP_CMDID_GET_ATTR  DPBP_CMD(0x004)
> +#define DPBP_CMDID_RESET DPBP_CMD(0x005)
> +
> +struct dpbp_cmd_open {
> + __le32 dpbp_id;
> +};
> +
> +#define DPBP_ENABLE  0x1
> +
> +struct dpbp_rsp_get_attributes {
> + /* response word 0 */
> + __le16 pad;
> + __le16 bpid;
> + __le32 id;
> + /* response word 1 */
> + __le16 version_major;
> + __le16 version_minor;
> +};
> +
>   /**
>* Maximum number of total IRQs that can be pre-allocated for an MC bus'
>* IRQ pool
> diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h 
> b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
> index e577410..ce864ee 100644
> --- a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
> +++ b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h
> @@ -35,10 +35,10 @@
>
>   #include 
>   #include 
> +#include

Re: [PATCH v6 0/6] staging: Introduce DPAA2 Ethernet Switch driver

[Laurentiu Tudor]

Hi Dan,

On 03/15/2018 12:56 PM, Dan Carpenter wrote:
> On Thu, Mar 15, 2018 at 12:44:37AM +0100, Andrew Lunn wrote:
>> On Wed, Mar 14, 2018 at 10:55:52AM -0500, Razvan Stefanescu wrote:
>>> This patchset introduces the Ethernet Switch Driver for Freescale/NXP SoCs
>>> with DPAA2 (DataPath Acceleration Architecture v2). The driver manages
>>> switch objects discovered on the fsl-mc bus. A description of the driver
>>> can be found in the associated README file.
>>
>> Hi Greg
>>
>> This code has much better quality than the usual stuff in staging. I
>> see no reason not to merge it.
>
> Yeah.  It seems pretty decent.  Stuart, Laurentiu, care to comment?

Not sure on what you want us to comment ...

> Meanwhile, netdev and DaveM aren't even on the CC list and they're the
> ones to ultimately decide.

I think we'll post to netdev when we'll be done with the TODOs
and start moving the driver out of staging.

---
Best Regards, Laurentiu

Re: [PATCH 5/6 v3] bus: fsl-mc: supoprt dma configure for devices on fsl-mc bus

[Laurentiu Tudor]

Hi Nipun,

On 04/27/2018 01:27 PM, Nipun Gupta wrote:
> Signed-off-by: Nipun Gupta 
> ---
>   drivers/bus/fsl-mc/fsl-mc-bus.c | 16 
>   1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c
> index 5d8266c..624828b 100644
> --- a/drivers/bus/fsl-mc/fsl-mc-bus.c
> +++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
> @@ -127,6 +127,16 @@ static int fsl_mc_bus_uevent(struct device *dev, struct 
> kobj_uevent_env *env)
>   return 0;
>   }
>
> +static int fsl_mc_dma_configure(struct device *dev)
> +{
> + struct device *dma_dev = dev;
> +
> + while (dev_is_fsl_mc(dma_dev))
> + dma_dev = dma_dev->parent;
> +
> + return of_dma_configure(dev, dma_dev->of_node, 0);
> +}
> +
>   static ssize_t modalias_show(struct device *dev, struct device_attribute 
> *attr,
>char *buf)
>   {
> @@ -148,6 +158,7 @@ struct bus_type fsl_mc_bus_type = {
>   .name = "fsl-mc",
>   .match = fsl_mc_bus_match,
>   .uevent = fsl_mc_bus_uevent,
> + .dma_configure  = fsl_mc_dma_configure,
>   .dev_groups = fsl_mc_dev_groups,
>   };
>   EXPORT_SYMBOL_GPL(fsl_mc_bus_type);
> @@ -616,6 +627,7 @@ int fsl_mc_device_add(struct fsl_mc_obj_desc *obj_desc,
>   mc_dev->icid = parent_mc_dev->icid;
>   mc_dev->dma_mask = FSL_MC_DEFAULT_DMA_MASK;
>   mc_dev->dev.dma_mask = _dev->dma_mask;
> + mc_dev->dev.coherent_dma_mask = mc_dev->dma_mask;

This change seems a bit unrelated to the patch subject. I wonder if it 
makes sense to split it it in a distinct patch.

---
Best Regards, Laurentiu

Re: [PATCH v4 6/6] arm64: dts: ls208xa: comply with the iommu map binding for fsl_mc

[Laurentiu Tudor]

Hi Nipun,

On 04/30/2018 09:27 AM, Nipun Gupta wrote:
> fsl-mc bus support the new iommu-map property. Comply to this binding
> for fsl_mc bus.
>
> Signed-off-by: Nipun Gupta 

This looks good to me, so:

Reviewed-By: Laurentiu Tudor 

---
Best Regards, Laurentiu

> ---
>   arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi | 6 +-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi 
> b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
> index 137ef4d..6010505 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
> @@ -184,6 +184,7 @@
>   #address-cells = <2>;
>   #size-cells = <2>;
>   ranges;
> + dma-ranges = <0x0 0x0 0x0 0x0 0x1 0x>;
>
>   clockgen: clocking@130 {
>   compatible = "fsl,ls2080a-clockgen";
> @@ -357,6 +358,8 @@
>   reg = <0x0008 0x0c00 0 0x40>,/* MC portal 
> base */
> <0x 0x0834 0 0x4>; /* MC control 
> reg */
>   msi-parent = <>;
> + iommu-map = <0  0 0>;  /* This is fixed-up by 
> u-boot */
> + dma-coherent;
>   #address-cells = <3>;
>   #size-cells = <1>;
>
> @@ -460,6 +463,8 @@
>   compatible = "arm,mmu-500";
>   reg = <0 0x500 0 0x80>;
>   #global-interrupts = <12>;
> + #iommu-cells = <1>;
> + stream-match-mask = <0x7C00>;
>   interrupts = <0 13 4>, /* global secure fault */
><0 14 4>, /* combined secure interrupt */
><0 15 4>, /* global non-secure fault */
> @@ -502,7 +507,6 @@
><0 204 4>, <0 205 4>,
><0 206 4>, <0 207 4>,
><0 208 4>, <0 209 4>;
> - mmu-masters = <_mc 0x300 0>;
>   };
>
>   dspi: dspi@210 {
>

Re: [PATCH v4 5/6] bus: fsl-mc: supoprt dma configure for devices on fsl-mc bus

[Laurentiu Tudor]

Hi Nipun,

On 04/30/2018 09:27 AM, Nipun Gupta wrote:
> Signed-off-by: Nipun Gupta 
> ---

If my understanding is correct, the kbuild error is triggered by this 
missing dependency patch:

https://patchwork.kernel.org/patch/10370081/

Apart from that, patch looks good to me, so

Reviewed-by: Laurentiu Tudor 

---
Best Regards, Laurentiu

>   drivers/bus/fsl-mc/fsl-mc-bus.c | 16 
>   1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c
> index 5d8266c..624828b 100644
> --- a/drivers/bus/fsl-mc/fsl-mc-bus.c
> +++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
> @@ -127,6 +127,16 @@ static int fsl_mc_bus_uevent(struct device *dev, struct 
> kobj_uevent_env *env)
>   return 0;
>   }
>
> +static int fsl_mc_dma_configure(struct device *dev)
> +{
> + struct device *dma_dev = dev;
> +
> + while (dev_is_fsl_mc(dma_dev))
> + dma_dev = dma_dev->parent;
> +
> + return of_dma_configure(dev, dma_dev->of_node, 0);
> +}
> +
>   static ssize_t modalias_show(struct device *dev, struct device_attribute 
> *attr,
>char *buf)
>   {
> @@ -148,6 +158,7 @@ struct bus_type fsl_mc_bus_type = {
>   .name = "fsl-mc",
>   .match = fsl_mc_bus_match,
>   .uevent = fsl_mc_bus_uevent,
> + .dma_configure  = fsl_mc_dma_configure,
>   .dev_groups = fsl_mc_dev_groups,
>   };
>   EXPORT_SYMBOL_GPL(fsl_mc_bus_type);
> @@ -616,6 +627,7 @@ int fsl_mc_device_add(struct fsl_mc_obj_desc *obj_desc,
>   mc_dev->icid = parent_mc_dev->icid;
>   mc_dev->dma_mask = FSL_MC_DEFAULT_DMA_MASK;
>   mc_dev->dev.dma_mask = _dev->dma_mask;
> + mc_dev->dev.coherent_dma_mask = mc_dev->dma_mask;
>   dev_set_msi_domain(_dev->dev,
>  dev_get_msi_domain(_mc_dev->dev));
>   }
> @@ -633,10 +645,6 @@ int fsl_mc_device_add(struct fsl_mc_obj_desc *obj_desc,
>   goto error_cleanup_dev;
>   }
>
> - /* Objects are coherent, unless 'no shareability' flag set. */
> - if (!(obj_desc->flags & FSL_MC_OBJ_FLAG_NO_MEM_SHAREABILITY))
> - arch_setup_dma_ops(_dev->dev, 0, 0, NULL, true);
> -
>   /*
>* The device-specific probe callback will get invoked by device_add()
>*/
>

Re: linux-next: build failure in the staging tree (Was: kisskb: FAILED linux-next/s390-allmodconfig/s390x Mon Jul 31, 17:24)

[Laurentiu Tudor]

Hi Stephen,

That's because the fsl-mc driver selects GENERIC_MSI_IRQ_DOMAIN and not 
all arches implement the support for the option. I can submit a patch 
that adds explicit dependencies on arches that it was build-tested (x86, 
arm, powerpc, all both 32 and 64 bits) similar to how it's done here 
[1]. Let me know if you're ok with this fix and i'll submit the fix to 
staging.

---
Thanks & Best Regards, Laurentiu

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/Kconfig#n28

On 07/31/2017 12:35 PM, Stephen Rothwell wrote:
> Hi all,
>
> On Mon, 31 Jul 2017 07:24:25 - nore...@ellerman.id.au wrote:
>>
>> FAILED linux-next/s390-allmodconfig/s390x Mon Jul 31, 17:24
>>
>> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fkisskb.ellerman.id.au%2Fkisskb%2Fbuildresult%2F13110910%2F=01%7C01%7Claurentiu.tudor%40nxp.com%7C0b58775ead154830f76f08d4d7f78d99%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0=A84bbMQe7VRz9OPK13PzaidBEfjm43Ik%2FbfCDJ89ZzM%3D=0
>>
>> Commit:   Add linux-next specific files for 20170731
>>742f220ee69c8102eabea45e45d92aa18721fab1
>> Compiler: s390x-linux-gcc (GCC) 4.6.3
>>
>> Possible errors
>> ---
>>
>> include/linux/msi.h:196:21: fatal error: asm/msi.h: No such file or directory
>> make[2]: *** [arch/s390/kernel/asm-offsets.s] Error 1
>> make[1]: *** [prepare0] Error 2
>> make: *** [sub-make] Error 2
>
> Caused by commit
>
>03274850279c ("staging: fsl-mc: allow the driver compile multi-arch")
>
> I will revert that commit tomorrow unless it if fixed in the mean time.
>

Re: linux-next: build failure in the staging tree (Was: kisskb: FAILED linux-next/s390-allmodconfig/s390x Mon Jul 31, 17:24)

[Laurentiu Tudor]

On 07/31/2017 06:58 PM, Greg KH wrote:
> On Mon, Jul 31, 2017 at 09:55:14AM +0000, Laurentiu Tudor wrote:
>> Hi Stephen,
>>
>> That's because the fsl-mc driver selects GENERIC_MSI_IRQ_DOMAIN and not
>> all arches implement the support for the option. I can submit a patch
>> that adds explicit dependencies on arches that it was build-tested (x86,
>> arm, powerpc, all both 32 and 64 bits) similar to how it's done here
>> [1]. Let me know if you're ok with this fix and i'll submit the fix to
>> staging.
>
> Ugh, you should not be selecting that option, but rather depending on
> the option, right?

All users in the kernel use "select", so i don't think so. An 
interesting use that adds explicit dependencies on architectures can be
seen here [1], in the generic code. I've proposed a patch [2] that does 
a similar thing for mc-bus. I think it's a good approach as it keeps 
things under control by explicitly specifying the architectures on which
the driver was compile-tested.

---
Best Regards, Laurentiu

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/Kconfig#n28
[2] https://patchwork.kernel.org/patch/9871861/

Re: [RESEND PATCH 2/6] staging: fsl-dpaa2/ethsw: Add Freescale DPAA2 Ethernet Switch driver

[Laurentiu Tudor]

Hi Florian,

On 09/29/2017 07:11 PM, Florian Fainelli wrote:
> On September 29, 2017 6:59:18 AM PDT, Razvan Stefanescu 
>  wrote:
>>
>>
>>> -Original Message-
>>> From: Bogdan Purcareata
>>> Sent: Friday, September 29, 2017 16:36
>>> To: Razvan Stefanescu ;
>>> gre...@linuxfoundation.org
>>> Cc: de...@driverdev.osuosl.org; linux-kernel@vger.kernel.org;
>>> net...@vger.kernel.org; ag...@suse.de; a...@arndb.de; Alexandru
>> Marginean
>>> ; Ruxandra Ioana Radulescu
>>> ; Laurentiu Tudor
>> ;
>>> stuyo...@gmail.com
>>> Subject: RE: [RESEND PATCH 2/6] staging: fsl-dpaa2/ethsw: Add
>> Freescale DPAA2
>>> Ethernet Switch driver
>>>
>>>> Introduce the DPAA2 Ethernet Switch driver, which manages Datapath
>> Switch
>>>> (DPSW) objects discovered on the MC bus.
>>>>
>>>> Suggested-by: Alexandru Marginean 
>>>> Signed-off-by: Razvan Stefanescu 
>
> This looks pretty good for a new switchdev driver, is there a reason you 
> can't target drivers/net/ethernet instead of staging? Is it because the MC 
> bus code is still in staging (AFAICT)?

There's a pending patch [1] that moves the bus from staging to its place 
in drivers/bus. DPIO plus other bits and pieces are next on the list.

Greg,

Just a heads up: if this driver goes in first, then i'll need to re-spin 
my patch [1] to also update the #include's in this switch driver.

P.S. Any news on my patch? :-)

[1] https://www.spinics.net/lists/arm-kernel/msg603534.html

---
Thanks, Laurentiu

Re: [PATCH v4] staging: fsl-mc: move bus driver out of staging

[Laurentiu Tudor]

On 12/19/2017 04:48 PM, Greg KH wrote:
> On Wed, Nov 29, 2017 at 12:08:44PM +0200, laurentiu.tu...@nxp.com wrote:
>> From: Stuart Yoder 
>>
>> Move the source files out of staging into their final locations:
>>-include files in drivers/staging/fsl-mc/include go to include/linux/fsl
>>-irq-gic-v3-its-fsl-mc-msi.c goes to drivers/irqchip
>>-source in drivers/staging/fsl-mc/bus goes to drivers/bus/fsl-mc
>>-README.txt, providing and overview of DPAA goes to
>> Documentation/dpaa2/overview.txt
>>
>> Update or delete other remaining staging files-- Makefile, Kconfig, TODO.
>> Update dpaa2_eth and dpio staging drivers.
>>
>> Signed-off-by: Stuart Yoder 
>> Signed-off-by: Laurentiu Tudor 
>> [Laurentiu: rebased, add dpaa2_eth and dpio #include updates]
>> Cc: Thomas Gleixner 
>> Cc: Jason Cooper 
>> Cc: Marc Zyngier 
>> ---
>> Notes:
>>  -v4:
>>- regenerated patch with renames detection disabled (Andrew Lunn)
>>  -v3:
>>- rebased
>
> Ok, meta-comments on the structure of the code.
>
> You have 8 .h files that are "private" to your bus logic.  That's 7 too
> many, some of them have a bigger license header than actual content :)
>
> Please consolidate into 1.
>
> Also, the headers should be moved to SPDX format to get rid of the
> boilerplate.  I _think_ it's BSD/GPL, right?  Hard to tell :(

It's 3-clause BSD and GPLv2. Will make it clear when moving to SPDX.

>
> Your "public" .h file does not need to go into a subdirectory, just name
> it fsl-mc.h and put it in include/linux/.

There's already a "fsl" subdirectory in include/linux/ so it seemed to 
make sense to use it.

> One comment on the fields in your .h file, all of the user/kernel
> crossing boundry structures need to use the "__" variant of types, like
> "__u8" and the like.  You mix and match them for some reason, you need
> to be consistent.
>
> Also, what's up with the .h files in drivers/staging/fsl-bus/include?
> You didn't touch those with this movement, right?  Why?

Those are not part of the bus "core". Some of them are part of the DPBP 
and DPCON device types APIs and are used by drivers probing on this bus 
and the rest are part of the DPIO driver which is also used by other 
drivers. Since these devices (DPBP, DPCON, DPIO) are interfaces used by 
all the other drivers it made sense to group them together with the bus.

> For this initial move, only move the bus "core" code out, not the other
> stuff like:
>
>>   drivers/irqchip/Makefile   |   1 +
>>   drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c| 119 +++
>
> these should be a separate file move, right?

This bus uses msi interrupts and this file contains glue code needed to 
enable interrupts in the GICv3 irqchip. Without this I don't think the 
bus driver can work because itself makes use of interrupts.

>>   drivers/staging/fsl-dpaa2/ethernet/README  |   2 +-
>
> Why does a README file for a different driver need to be touched?

It mentions a file in the old location of the bus. This is how the diff 
looks:

-   drivers/staging/fsl-mc/README.txt
+   Documentation/dpaa2/overview.txt


>>   drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c |   2 +-
>>   drivers/staging/fsl-dpaa2/ethernet/dpni.c  |   2 +-
>>   drivers/staging/fsl-mc/README.txt  | 386 -
>
> This file gets moved to the Documentation directory, yet it is not tied
> into the documentation build process, that's not good.

Will look into that.

> It doesn't need to have a separate directory either, right?

Agreed, maybe the destination directory isn't the best choice. Maybe 
bus-devices/fsl-mc.txt makes more sense? Can you please suggest?

> And speaking of documentation, you have directories in sysfs, yet no
> Documentation/ABI/ files describing them.  Please fix that up.

Hmm, I was under the impression that we did have sysfs documentation.
Will look into it.

> that's a good start :)

Yep. :)

---
Thanks & Best Regards, Laurentiu

Re: [PATCH v4] staging: fsl-mc: move bus driver out of staging

[Laurentiu Tudor]

On 12/19/2017 05:29 PM, Greg KH wrote:
> On Tue, Dec 19, 2017 at 03:21:19PM +0000, Laurentiu Tudor wrote:
>>
>>
>> On 12/19/2017 04:48 PM, Greg KH wrote:
>>> On Wed, Nov 29, 2017 at 12:08:44PM +0200, laurentiu.tu...@nxp.com wrote:
>>>> From: Stuart Yoder 
>>>>
>>>> Move the source files out of staging into their final locations:
>>>> -include files in drivers/staging/fsl-mc/include go to 
>>>> include/linux/fsl
>>>> -irq-gic-v3-its-fsl-mc-msi.c goes to drivers/irqchip
>>>> -source in drivers/staging/fsl-mc/bus goes to drivers/bus/fsl-mc
>>>> -README.txt, providing and overview of DPAA goes to
>>>>  Documentation/dpaa2/overview.txt
>>>>
>>>> Update or delete other remaining staging files-- Makefile, Kconfig, TODO.
>>>> Update dpaa2_eth and dpio staging drivers.
>>>>
>>>> Signed-off-by: Stuart Yoder 
>>>> Signed-off-by: Laurentiu Tudor 
>>>> [Laurentiu: rebased, add dpaa2_eth and dpio #include updates]
>>>> Cc: Thomas Gleixner 
>>>> Cc: Jason Cooper 
>>>> Cc: Marc Zyngier 
>>>> ---
>>>> Notes:
>>>>   -v4:
>>>> - regenerated patch with renames detection disabled (Andrew Lunn)
>>>>   -v3:
>>>> - rebased
>>>
>>> Ok, meta-comments on the structure of the code.
>>>
>>> You have 8 .h files that are "private" to your bus logic.  That's 7 too
>>> many, some of them have a bigger license header than actual content :)
>>>
>>> Please consolidate into 1.
>>>
>>> Also, the headers should be moved to SPDX format to get rid of the
>>> boilerplate.  I _think_ it's BSD/GPL, right?  Hard to tell :(
>>
>> It's 3-clause BSD and GPLv2. Will make it clear when moving to SPDX.
>
> Thanks.
>
>>> Your "public" .h file does not need to go into a subdirectory, just name
>>> it fsl-mc.h and put it in include/linux/.
>>
>> There's already a "fsl" subdirectory in include/linux/ so it seemed to
>> make sense to use it.
>
> Ah, missed that.  Ok, nevermind :)`
>
>>> One comment on the fields in your .h file, all of the user/kernel
>>> crossing boundry structures need to use the "__" variant of types, like
>>> "__u8" and the like.  You mix and match them for some reason, you need
>>> to be consistent.
>>>
>>> Also, what's up with the .h files in drivers/staging/fsl-bus/include?
>>> You didn't touch those with this movement, right?  Why?
>>
>> Those are not part of the bus "core". Some of them are part of the DPBP
>> and DPCON device types APIs and are used by drivers probing on this bus
>> and the rest are part of the DPIO driver which is also used by other
>> drivers. Since these devices (DPBP, DPCON, DPIO) are interfaces used by
>> all the other drivers it made sense to group them together with the bus.
>
> But all of these .h files are only used by the code in this specific
> directory, no where else.

They are also used by our ethernet driver, see:
   drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h

> So just mush them all together, having
> individual .h files doesn't really help anyone here, right?  It's just
> more files for no good reason.  And, it might help you see if you really
> need all of the info in those files :)

Ok, I'll try to come up with something that reduces the number of header 
files.

>>> For this initial move, only move the bus "core" code out, not the other
>>> stuff like:
>>>
>>>>drivers/irqchip/Makefile   |   1 +
>>>>drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c| 119 +++
>>>
>>> these should be a separate file move, right?
>>
>> This bus uses msi interrupts and this file contains glue code needed to
>> enable interrupts in the GICv3 irqchip. Without this I don't think the
>> bus driver can work because itself makes use of interrupts.
>
> How is this all working today?  Just leave the non-bus code alone, and
> move the irqchip code as a separate patch.

Ok, i can do this.

>>>>drivers/staging/fsl-dpaa2/ethernet/README  |   2 +-
>>>
>>> Why does a README file for a different driver need to be touched?
>>
>> It mentions a file in the old location of the bus. This is how the diff
>> looks:
>>
>> -   drivers/staging/fsl-mc/README.txt
>> +   Documentation/dpaa2/overview.txt
>
> Ah.
>
>>>>drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c |   2 +-
>>>>drivers/staging/fsl-dpaa2/ethernet/dpni.c  |   2 +-
>>>>drivers/staging/fsl-mc/README.txt  | 386 -
>>>
>>> This file gets moved to the Documentation directory, yet it is not tied
>>> into the documentation build process, that's not good.
>>
>> Will look into that.
>>
>>> It doesn't need to have a separate directory either, right?
>>
>> Agreed, maybe the destination directory isn't the best choice. Maybe
>> bus-devices/fsl-mc.txt makes more sense? Can you please suggest?
>
> It should be in .rst format, and tied into the documentation build
> somehow, I don't really know where new stuff is going, you should look
> at recent changes in that directory for more examples.

Ok, will do.

---
Thanks & Best Regards, Laurentiu