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

2018-10-02 Thread 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);
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2018-07-09 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2018-03-16 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2018-03-02 Thread 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
> @

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

2017-12-20 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-12-20 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-12-20 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-12-19 Thread 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

2017-12-19 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-12-07 Thread 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-ker...@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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-12-06 Thread 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-ker...@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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-12-06 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-11-29 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-11-28 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-11-27 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-11-27 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [staging:staging-testing 104/105] drivers/staging//fsl-mc/bus/fsl-mc-bus.c:663:9: warning: 'error' may be used uninitialized in this function

2017-11-27 Thread Laurentiu Tudor
Hi Greg,

On 11/27/2017 10:28 AM, Greg Kroah-Hartman wrote:
> On Fri, Nov 24, 2017 at 06:02:08PM +0100, Greg Kroah-Hartman wrote:
>> On Sat, Nov 25, 2017 at 01:01:18AM +0800, kbuild test robot wrote:
>>> tree:   
>>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fgregkh%2Fstaging.git=02%7C01%7Claurentiu.tudor%40nxp.com%7C065154b2dff543c047e808d53570e3f9%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636473681334394349=nsL1h3p2CKdH9yeQs%2FC0Pxf4e2xaLkRWZfKjCt0PMvo%3D=0
>>>  staging-testing
>>> head:   f3145c023966af8e207b6b3cdb6187c35e7f9e95
>>> commit: 657582b9e91728b91307ef6ba6f069ed0b67c7b4 [104/105] staging: fsl-mc: 
>>> add support for device type
>>> config: powerpc-allyesconfig (attached as .config)
>>> compiler: powerpc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
>>> reproduce:
>>>  wget 
>>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fraw.githubusercontent.com%2Fintel%2Flkp-tests%2Fmaster%2Fsbin%2Fmake.cross=02%7C01%7Claurentiu.tudor%40nxp.com%7C065154b2dff543c047e808d53570e3f9%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636473681334394349=VSdLCde%2FPO8L4U17MUzZX%2BOmmLdRJAonOijgZAUFN3M%3D=0
>>>  -O ~/bin/make.cross
>>>  chmod +x ~/bin/make.cross
>>>  git checkout 657582b9e91728b91307ef6ba6f069ed0b67c7b4
>>>  # save the attached .config to linux build tree
>>>  make.cross ARCH=powerpc
>>>
>>> Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
>>> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fgcc.gnu.org%2Fwiki%2FBetter_Uninitialized_Warnings=02%7C01%7Claurentiu.tudor%40nxp.com%7C065154b2dff543c047e808d53570e3f9%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636473681334394349=bUz%2B9aFSbX0XiT8uHquoCipid3%2FFwLzAkhZKVBYZzGw%3D=0
>>
>> Looks like a real warning, Laurentiu, can you provide a patch to fix
>> this?
>
> Nevermind, I've now fixed it up :(
>

Seen this only after i send a patch, please disregard it. Thanks for 
taking care of the warning.

---
Best Regards, Laurentiu
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-11-22 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-11-06 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-11-03 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-10-02 Thread 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-ker...@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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-09-11 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-09-11 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-08-23 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-08-16 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-07-20 Thread 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 v2 0/7] staging: fsl-mc: make the driver compile on other architectures

2017-07-18 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-07-18 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-07-18 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-07-18 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-07-17 Thread 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);
>>   }
>>
>>   /**
>>
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-07-17 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-07-17 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


RE: [staging:staging-testing 357/367] make[5]: *** No rule to make target 'drivers/staging/fsl-mc/bus/dpmng.o', needed by 'drivers/staging/fsl-mc/bus/mc-bus-driver.o'.

2017-06-26 Thread Laurentiu Tudor


> -Original Message-
> From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org]
> Sent: Monday, June 26, 2017 2:55 PM
> To: Laurentiu Tudor <laurentiu.tu...@nxp.com>
> 
> On Mon, Jun 26, 2017 at 01:48:01PM +0200, Greg Kroah-Hartman wrote:
> > On Mon, Jun 26, 2017 at 09:06:11AM +, Laurentiu Tudor wrote:
> > > Hi Greg,
> > >
> > > > -Original Message-
> > > > From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org]
> > > > Sent: Monday, June 26, 2017 11:50 AM
> > > > To: Laurentiu Tudor <laurentiu.tu...@nxp.com>
> > > >
> > > > On Mon, Jun 26, 2017 at 08:39:20AM +, Laurentiu Tudor wrote:
> > > > > Hi Greg,
> > > > >
> > > > > > -Original Message-
> > > > > > From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org]
> > > > > >
> > > > > > On Sat, Jun 24, 2017 at 08:17:59PM +0800, kbuild test robot wrote:
> > > > > > > tree:
> > > > > > https://emea01.safelinks.protection.outlook.com/?url=https%3A%
> > > > > > 2F%2Fg
> > > > > > it.ker
> > > > > >
> > > >
> nel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fgregkh%2Fstaging.git&
> > > > da
> > > > > > ta
> > > > > >
> > > >
> =01%7C01%7Claurentiu.tudor%40nxp.com%7C751676566b8f465b5fa108d4bb1
> > > > > >
> > > >
> 69974%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0=hDMOrVmq8h6I
> > > > > > MBDuCHgARD44vX58GOPYWIQSqMsdAjo%3D=0 staging-
> testing
> > > > > > > head:   18cd9021ea035db85519391dbc429a5b1d0dd25b
> > > > > > > commit: b065307fe0ad7859f01ce8560e6bdc590324561a [357/367]
> > > > staging:
> > > > > > > fsl-mc: remove dpmng API files
> > > > > > > config: arm64-allyesconfig (attached as .config)
> > > > > > > compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1
> > > > > > > 20160705
> > > > > > > reproduce:
> > > > > > > wget
> > > > > > https://emea01.safelinks.protection.outlook.com/?url=https%3A%
> > > > > > 2F%2Fr
> > > > > > aw.git
> > > > > > hubusercontent.com%2F01org%2Flkp-
> > > > > >
> > > >
> tests%2Fmaster%2Fsbin%2Fmake.cross=01%7C01%7Claurentiu.tudor%
> > > > 40
> > > > > >
> > > >
> nxp.com%7C751676566b8f465b5fa108d4bb169974%7C686ea1d3bc2b4c6fa92c
> > > > > >
> > > >
> d99c5c301635%7C0=JvBQcG8MAs7PwDoXxYXe89sqs%2B2ADAiQd7sYCq
> > > > > > 0YbPE%3D=0 -O ~/bin/make.cross
> > > > > > > chmod +x ~/bin/make.cross
> > > > > > > git checkout b065307fe0ad7859f01ce8560e6bdc590324561a
> > > > > > > # save the attached .config to linux build tree
> > > > > > > make.cross ARCH=arm64
> > > > > > >
> > > > > > > All errors (new ones prefixed by >>):
> > > > > > >
> > > > > > > >> make[5]: *** No rule to make target
> > > > > > > >> 'drivers/staging/fsl-mc/bus/dpmng.o',
> > > > > > needed by 'drivers/staging/fsl-mc/bus/mc-bus-driver.o'.
> > > > > > >make[5]: Target '__build' not remade because of errors.
> > > > > >
> > > > > > Crap, this isn't good.  But it looks like further patches fixes 
> > > > > > this issue,
> right?
> > > > >
> > > > > Wow, doesn't look like so. I don't know how I managed to screw
> > > > > this up so badly. :-(
> > > > >
> > > > > > Laurentiu, any ideas here?  You do always test every
> > > > > > individual patch, right? :)
> > > > >
> > > > > Usually yes, but looks like this time I managed to mess it up.
> > > > > Sorry for the
> > > > trouble.
> > > > >
> > > > > > Note, if you all ever got multi-arch building working for this
> > > > > > code, I would have caught this a lot earlier, that should be
> > > > > > something you fix up to get this out of staging, no reason to only
> depend on one arch.
> > > > >
> > > > > Got it. I don't think there's any arch depended stuff in the
> > > > > code so there
> > > > shouldn't be issues.
> > > > > I'll fix up this series and submit a v2 of the whole patch set
> > > > > and after that look
> > > > into the multi-arch stuff.
> > > > > Is this ok?
> > > >
> > > > No, it's already in my tree, so either I revert the patch series,
> > > > or I take a fixup patch for it.  Which do you want me to do?
> > >
> > > Let's go with reverting the whole series and me to resubmit a v2
> > > because there are also a couple of comments from Joe Perches that
> > > I'd like to include.
> >
> > Ugh, ok, will do...
> >
> 
> Now all reverted and pushed out for 0-day to run on it.
> 
> Please be more careful in the future.

Will do, sorry again for all the trouble.

BTW, I think I found out what happened: I didn't try a clean build. :-(
I'll make sure to include this in my pre-submission validations.

---
Best Regards, Laurentiu
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


RE: [staging:staging-testing 357/367] make[5]: *** No rule to make target 'drivers/staging/fsl-mc/bus/dpmng.o', needed by 'drivers/staging/fsl-mc/bus/mc-bus-driver.o'.

2017-06-26 Thread Laurentiu Tudor
Hi Greg,

> -Original Message-
> From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org]
> Sent: Monday, June 26, 2017 11:50 AM
> To: Laurentiu Tudor <laurentiu.tu...@nxp.com>
> 
> On Mon, Jun 26, 2017 at 08:39:20AM +, Laurentiu Tudor wrote:
> > Hi Greg,
> >
> > > -Original Message-
> > > From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org]
> > >
> > > On Sat, Jun 24, 2017 at 08:17:59PM +0800, kbuild test robot wrote:
> > > > tree:
> > > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fg
> > > it.ker
> > >
> nel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fgregkh%2Fstaging.git
> > > ta
> > >
> =01%7C01%7Claurentiu.tudor%40nxp.com%7C751676566b8f465b5fa108d4bb1
> > >
> 69974%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0=hDMOrVmq8h6I
> > > MBDuCHgARD44vX58GOPYWIQSqMsdAjo%3D=0 staging-testing
> > > > head:   18cd9021ea035db85519391dbc429a5b1d0dd25b
> > > > commit: b065307fe0ad7859f01ce8560e6bdc590324561a [357/367]
> staging:
> > > > fsl-mc: remove dpmng API files
> > > > config: arm64-allyesconfig (attached as .config)
> > > > compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
> > > > reproduce:
> > > > wget
> > > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fr
> > > aw.git
> > > hubusercontent.com%2F01org%2Flkp-
> > >
> tests%2Fmaster%2Fsbin%2Fmake.cross=01%7C01%7Claurentiu.tudor%40
> > >
> nxp.com%7C751676566b8f465b5fa108d4bb169974%7C686ea1d3bc2b4c6fa92c
> > >
> d99c5c301635%7C0=JvBQcG8MAs7PwDoXxYXe89sqs%2B2ADAiQd7sYCq
> > > 0YbPE%3D=0 -O ~/bin/make.cross
> > > > chmod +x ~/bin/make.cross
> > > > git checkout b065307fe0ad7859f01ce8560e6bdc590324561a
> > > > # save the attached .config to linux build tree
> > > > make.cross ARCH=arm64
> > > >
> > > > All errors (new ones prefixed by >>):
> > > >
> > > > >> make[5]: *** No rule to make target
> > > > >> 'drivers/staging/fsl-mc/bus/dpmng.o',
> > > needed by 'drivers/staging/fsl-mc/bus/mc-bus-driver.o'.
> > > >make[5]: Target '__build' not remade because of errors.
> > >
> > > Crap, this isn't good.  But it looks like further patches fixes this 
> > > issue, right?
> >
> > Wow, doesn't look like so. I don't know how I managed to screw this up
> > so badly. :-(
> >
> > > Laurentiu, any ideas here?  You do always test every individual
> > > patch, right? :)
> >
> > Usually yes, but looks like this time I managed to mess it up. Sorry for the
> trouble.
> >
> > > Note, if you all ever got multi-arch building working for this code,
> > > I would have caught this a lot earlier, that should be something you
> > > fix up to get this out of staging, no reason to only depend on one arch.
> >
> > Got it. I don't think there's any arch depended stuff in the code so there
> shouldn't be issues.
> > I'll fix up this series and submit a v2 of the whole patch set and after 
> > that look
> into the multi-arch stuff.
> > Is this ok?
> 
> No, it's already in my tree, so either I revert the patch series, or I take a 
> fixup
> patch for it.  Which do you want me to do?

Let's go with reverting the whole series and me to resubmit a v2 because there 
are also a couple of comments from Joe Perches that I'd like to include.

---
Thanks & Best Regards, Laurentiu
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


RE: [staging:staging-testing 357/367] make[5]: *** No rule to make target 'drivers/staging/fsl-mc/bus/dpmng.o', needed by 'drivers/staging/fsl-mc/bus/mc-bus-driver.o'.

2017-06-26 Thread Laurentiu Tudor
Hi Greg,

> -Original Message-
> From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org]
> 
> On Sat, Jun 24, 2017 at 08:17:59PM +0800, kbuild test robot wrote:
> > tree:
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.ker
> nel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fgregkh%2Fstaging.git
> =01%7C01%7Claurentiu.tudor%40nxp.com%7C751676566b8f465b5fa108d4bb1
> 69974%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0=hDMOrVmq8h6I
> MBDuCHgARD44vX58GOPYWIQSqMsdAjo%3D=0 staging-testing
> > head:   18cd9021ea035db85519391dbc429a5b1d0dd25b
> > commit: b065307fe0ad7859f01ce8560e6bdc590324561a [357/367] staging:
> > fsl-mc: remove dpmng API files
> > config: arm64-allyesconfig (attached as .config)
> > compiler: aarch64-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.git
> hubusercontent.com%2F01org%2Flkp-
> tests%2Fmaster%2Fsbin%2Fmake.cross=01%7C01%7Claurentiu.tudor%40
> nxp.com%7C751676566b8f465b5fa108d4bb169974%7C686ea1d3bc2b4c6fa92c
> d99c5c301635%7C0=JvBQcG8MAs7PwDoXxYXe89sqs%2B2ADAiQd7sYCq
> 0YbPE%3D=0 -O ~/bin/make.cross
> > chmod +x ~/bin/make.cross
> > git checkout b065307fe0ad7859f01ce8560e6bdc590324561a
> > # save the attached .config to linux build tree
> > make.cross ARCH=arm64
> >
> > All errors (new ones prefixed by >>):
> >
> > >> make[5]: *** No rule to make target 'drivers/staging/fsl-mc/bus/dpmng.o',
> needed by 'drivers/staging/fsl-mc/bus/mc-bus-driver.o'.
> >make[5]: Target '__build' not remade because of errors.
> 
> Crap, this isn't good.  But it looks like further patches fixes this issue, 
> right?

Wow, doesn't look like so. I don't know how I managed to screw this up so 
badly. :-(

> Laurentiu, any ideas here?  You do always test every individual patch, right? 
> :)

Usually yes, but looks like this time I managed to mess it up. Sorry for the 
trouble.

> Note, if you all ever got multi-arch building working for this code, I would 
> have
> caught this a lot earlier, that should be something you fix up to get this 
> out of
> staging, no reason to only depend on one arch.

Got it. I don't think there's any arch depended stuff in the code so there 
shouldn't be issues.
I'll fix up this series and submit a v2 of the whole patch set and after that 
look into the multi-arch stuff.
Is this ok?

---
Thanks & Best Regards, Laurentiu
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-06-23 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-06-14 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-06-14 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-06-06 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-06-06 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-06-06 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-05-31 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-05-29 Thread 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-ker...@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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-05-26 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-05-24 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-05-22 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-05-22 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-05-22 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-05-17 Thread 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-ker...@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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-02-28 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-02-15 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-02-03 Thread 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-ker...@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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-02-03 Thread 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-ker...@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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2017-02-03 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2016-12-06 Thread 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2016-12-02 Thread 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: [PATCH v3 7/9] bus: fsl-mc: dpio: add the DPAA2 DPIO service interface

2016-12-02 Thread 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 6/9] bus: fsl-mc: dpio: add QBMan portal APIs for DPAA2

2016-12-02 Thread 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 5/9] bus: fsl-mc: dpio: add global dpaa2 definitions

2016-12-02 Thread 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 4/9] bus: fsl-mc: dpio: add frame descriptor and scatter/gather APIs

2016-12-02 Thread 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