RE: [PATCH 00/14] staging: fsl-mc: misc updates

[Jose Rivera]

> -Original Message-
> From: Stuart Yoder [mailto:stuart.yo...@nxp.com]
> Sent: Monday, April 11, 2016 11:48 AM
> To: gre...@linuxfoundation.org; Jose Rivera <german.riv...@nxp.com>
> Cc: de...@driverdev.osuosl.org; linux-kernel@vger.kernel.org; ag...@suse.de;
> a...@arndb.de; Yang-Leo Li <leoyang...@nxp.com>; Stuart Yoder
> <stuart.yo...@nxp.com>
> Subject: [PATCH 00/14] staging: fsl-mc: misc updates
> 
> From: Stuart Yoder <stuart.yo...@nxp.com>
> 
> This patch series makes further progress towards completing the fsl-mc TODO
> list.
> 
> -patch 1 removes three items from the TODO file that were previously
>  completed-- multiple root dprc support, MSI support, and command
> serialization
> 
> -patch 2 makes some way overdue updates to README.txt from review
> comments on the mailing list last fall
> 
> -patches 3-5 update the binary interface for several objects to  sync with 
> latest
> MC firmware (todo item: "MC firmware uprev")
> 
> -patches 6-13 are cleanup items
>-change in how object versions are used in binding decisions
>-setting coherent dma_ops for devices if necessary
>-setting cacheable flag for object regions if applicable
>-getting the version of the root dprc from hardware, not based
> on what was in the .h file
>-dprc driver now refuses to probe if dprc version is not minimum
> expected
>-added quirk for bug in coherency flag for dpseci objects
>-allocator driver refuses to bind to dpmcp objects if version
> is not minimum expected
> 
> -patch 14 adds a second maintainer for the driver
> 
> Horia Geanta (1):
>   staging: fsl-mc: add quirk handling for dpseci objects < 4.0
> 
> Horia Geantă (1):
>   staging: fsl-mc: return -EINVAL for all fsl_mc_portal_allocate()
> failures
> 
> Itai Katz (5):
>   staging: fsl-mc: don't use object versions to make binding decisions
>   staging: fsl-mc: set cacheable flag for added devices if applicable
>   staging: fsl-mc: get version of root dprc from MC hardware
>   staging: fsl-mc: add dprc version check
>   staging: fsl-mc: add dpmcp version check
> 
> Stuart Yoder (7):
>   staging: fsl-mc: TODO updates
>   staging: fsl-mc: DPAA2 overview readme update
>   staging: fsl-mc: update dpmcp binary interface to v3.0
>   staging: fsl-mc: update dpbp binary interface to v2.2
>   staging: fsl-mc: update dprc binary interface to v5.1
>   staging: fsl-mc: set up coherent dma ops for added devices
>   MAINTAINERS: fsl-mc: Add second maintainer
> 
>  MAINTAINERS |1 +
>  drivers/staging/fsl-mc/README.txt   |  138 
> ---
>  drivers/staging/fsl-mc/TODO |   13 ---
>  drivers/staging/fsl-mc/bus/dpbp.c   |   77 ++-
>  drivers/staging/fsl-mc/bus/dpmcp-cmd.h  |7 +-
>  drivers/staging/fsl-mc/bus/dpmcp.c  |   35 +--
>  drivers/staging/fsl-mc/bus/dpmcp.h  |   10 +-
>  drivers/staging/fsl-mc/bus/dprc-cmd.h   |6 +-
>  drivers/staging/fsl-mc/bus/dprc-driver.c|   33 ++-
>  drivers/staging/fsl-mc/bus/dprc.c   |   26 ++---
>  drivers/staging/fsl-mc/bus/mc-allocator.c   |   18 ++--
>  drivers/staging/fsl-mc/bus/mc-bus.c |   93 +-
>  drivers/staging/fsl-mc/bus/mc-msi.c |2 +-
>  drivers/staging/fsl-mc/include/dpbp-cmd.h   |4 +-
>  drivers/staging/fsl-mc/include/dpbp.h   |   51 +-
>  drivers/staging/fsl-mc/include/dprc.h   |   19 ++--
>  drivers/staging/fsl-mc/include/mc-private.h |2 +
>  17 files changed, 335 insertions(+), 200 deletions(-)
> 
> --
> 1.7.9.5

For the series:
 Acked-by: German Rivera <german.riv...@nxp.com>

RE: [PATCH 00/14] staging: fsl-mc: misc updates

[Jose Rivera]

> -Original Message-
> From: Stuart Yoder [mailto:stuart.yo...@nxp.com]
> Sent: Monday, April 11, 2016 11:48 AM
> To: gre...@linuxfoundation.org; Jose Rivera 
> Cc: de...@driverdev.osuosl.org; linux-kernel@vger.kernel.org; ag...@suse.de;
> a...@arndb.de; Yang-Leo Li ; Stuart Yoder
> 
> Subject: [PATCH 00/14] staging: fsl-mc: misc updates
> 
> From: Stuart Yoder 
> 
> This patch series makes further progress towards completing the fsl-mc TODO
> list.
> 
> -patch 1 removes three items from the TODO file that were previously
>  completed-- multiple root dprc support, MSI support, and command
> serialization
> 
> -patch 2 makes some way overdue updates to README.txt from review
> comments on the mailing list last fall
> 
> -patches 3-5 update the binary interface for several objects to  sync with 
> latest
> MC firmware (todo item: "MC firmware uprev")
> 
> -patches 6-13 are cleanup items
>-change in how object versions are used in binding decisions
>-setting coherent dma_ops for devices if necessary
>-setting cacheable flag for object regions if applicable
>-getting the version of the root dprc from hardware, not based
> on what was in the .h file
>-dprc driver now refuses to probe if dprc version is not minimum
> expected
>-added quirk for bug in coherency flag for dpseci objects
>-allocator driver refuses to bind to dpmcp objects if version
> is not minimum expected
> 
> -patch 14 adds a second maintainer for the driver
> 
> Horia Geanta (1):
>   staging: fsl-mc: add quirk handling for dpseci objects < 4.0
> 
> Horia Geantă (1):
>   staging: fsl-mc: return -EINVAL for all fsl_mc_portal_allocate()
> failures
> 
> Itai Katz (5):
>   staging: fsl-mc: don't use object versions to make binding decisions
>   staging: fsl-mc: set cacheable flag for added devices if applicable
>   staging: fsl-mc: get version of root dprc from MC hardware
>   staging: fsl-mc: add dprc version check
>   staging: fsl-mc: add dpmcp version check
> 
> Stuart Yoder (7):
>   staging: fsl-mc: TODO updates
>   staging: fsl-mc: DPAA2 overview readme update
>   staging: fsl-mc: update dpmcp binary interface to v3.0
>   staging: fsl-mc: update dpbp binary interface to v2.2
>   staging: fsl-mc: update dprc binary interface to v5.1
>   staging: fsl-mc: set up coherent dma ops for added devices
>   MAINTAINERS: fsl-mc: Add second maintainer
> 
>  MAINTAINERS |1 +
>  drivers/staging/fsl-mc/README.txt   |  138 
> ---
>  drivers/staging/fsl-mc/TODO |   13 ---
>  drivers/staging/fsl-mc/bus/dpbp.c   |   77 ++-
>  drivers/staging/fsl-mc/bus/dpmcp-cmd.h  |7 +-
>  drivers/staging/fsl-mc/bus/dpmcp.c  |   35 +--
>  drivers/staging/fsl-mc/bus/dpmcp.h  |   10 +-
>  drivers/staging/fsl-mc/bus/dprc-cmd.h   |6 +-
>  drivers/staging/fsl-mc/bus/dprc-driver.c|   33 ++-
>  drivers/staging/fsl-mc/bus/dprc.c   |   26 ++---
>  drivers/staging/fsl-mc/bus/mc-allocator.c   |   18 ++--
>  drivers/staging/fsl-mc/bus/mc-bus.c |   93 +-
>  drivers/staging/fsl-mc/bus/mc-msi.c |2 +-
>  drivers/staging/fsl-mc/include/dpbp-cmd.h   |4 +-
>  drivers/staging/fsl-mc/include/dpbp.h   |   51 +-
>  drivers/staging/fsl-mc/include/dprc.h   |   19 ++--
>  drivers/staging/fsl-mc/include/mc-private.h |2 +
>  17 files changed, 335 insertions(+), 200 deletions(-)
> 
> --
> 1.7.9.5

For the series:
 Acked-by: German Rivera 

RE: [PATCH 00/14] staging: fsl-mc: misc updates

[Jose Rivera]

> -Original Message-
> From: Stuart Yoder [mailto:stuart.yo...@nxp.com]
> Sent: Monday, April 11, 2016 11:48 AM
> To: gre...@linuxfoundation.org; Jose Rivera <german.riv...@nxp.com>
> Cc: de...@driverdev.osuosl.org; linux-kernel@vger.kernel.org; ag...@suse.de;
> a...@arndb.de; Yang-Leo Li <leoyang...@nxp.com>; Stuart Yoder
> <stuart.yo...@nxp.com>
> Subject: [PATCH 00/14] staging: fsl-mc: misc updates
> 
> From: Stuart Yoder <stuart.yo...@nxp.com>
> 
> This patch series makes further progress towards completing the fsl-mc TODO
> list.
> 
> -patch 1 removes three items from the TODO file that were previously
>  completed-- multiple root dprc support, MSI support, and command
> serialization
> 
> -patch 2 makes some way overdue updates to README.txt from review
> comments on the mailing list last fall
> 
> -patches 3-5 update the binary interface for several objects to  sync with 
> latest
> MC firmware (todo item: "MC firmware uprev")
> 
> -patches 6-13 are cleanup items
>-change in how object versions are used in binding decisions
>-setting coherent dma_ops for devices if necessary
>-setting cacheable flag for object regions if applicable
>-getting the version of the root dprc from hardware, not based
> on what was in the .h file
>-dprc driver now refuses to probe if dprc version is not minimum
> expected
>-added quirk for bug in coherency flag for dpseci objects
>-allocator driver refuses to bind to dpmcp objects if version
> is not minimum expected
> 
> -patch 14 adds a second maintainer for the driver
> 
> Horia Geanta (1):
>   staging: fsl-mc: add quirk handling for dpseci objects < 4.0
> 
> Horia Geantă (1):
>   staging: fsl-mc: return -EINVAL for all fsl_mc_portal_allocate()
> failures
> 
> Itai Katz (5):
>   staging: fsl-mc: don't use object versions to make binding decisions
>   staging: fsl-mc: set cacheable flag for added devices if applicable
>   staging: fsl-mc: get version of root dprc from MC hardware
>   staging: fsl-mc: add dprc version check
>   staging: fsl-mc: add dpmcp version check
> 
> Stuart Yoder (7):
>   staging: fsl-mc: TODO updates
>   staging: fsl-mc: DPAA2 overview readme update
>   staging: fsl-mc: update dpmcp binary interface to v3.0
>   staging: fsl-mc: update dpbp binary interface to v2.2
>   staging: fsl-mc: update dprc binary interface to v5.1
>   staging: fsl-mc: set up coherent dma ops for added devices
>   MAINTAINERS: fsl-mc: Add second maintainer
> 
>  MAINTAINERS |1 +
>  drivers/staging/fsl-mc/README.txt   |  138 
> ---
>  drivers/staging/fsl-mc/TODO |   13 ---
>  drivers/staging/fsl-mc/bus/dpbp.c   |   77 ++-
>  drivers/staging/fsl-mc/bus/dpmcp-cmd.h  |7 +-
>  drivers/staging/fsl-mc/bus/dpmcp.c  |   35 +--
>  drivers/staging/fsl-mc/bus/dpmcp.h  |   10 +-
>  drivers/staging/fsl-mc/bus/dprc-cmd.h   |6 +-
>  drivers/staging/fsl-mc/bus/dprc-driver.c|   33 ++-
>  drivers/staging/fsl-mc/bus/dprc.c   |   26 ++---
>  drivers/staging/fsl-mc/bus/mc-allocator.c   |   18 ++--
>  drivers/staging/fsl-mc/bus/mc-bus.c |   93 +-
>  drivers/staging/fsl-mc/bus/mc-msi.c |2 +-
>  drivers/staging/fsl-mc/include/dpbp-cmd.h   |4 +-
>  drivers/staging/fsl-mc/include/dpbp.h   |   51 +-
>  drivers/staging/fsl-mc/include/dprc.h   |   19 ++--
>  drivers/staging/fsl-mc/include/mc-private.h |2 +
>  17 files changed, 335 insertions(+), 200 deletions(-)
> 
> --
> 1.7.9.5

Acked-by: German Rivera <german.riv...@nxp.com>

RE: [PATCH 00/14] staging: fsl-mc: misc updates

[Jose Rivera]

> -Original Message-
> From: Stuart Yoder [mailto:stuart.yo...@nxp.com]
> Sent: Monday, April 11, 2016 11:48 AM
> To: gre...@linuxfoundation.org; Jose Rivera 
> Cc: de...@driverdev.osuosl.org; linux-kernel@vger.kernel.org; ag...@suse.de;
> a...@arndb.de; Yang-Leo Li ; Stuart Yoder
> 
> Subject: [PATCH 00/14] staging: fsl-mc: misc updates
> 
> From: Stuart Yoder 
> 
> This patch series makes further progress towards completing the fsl-mc TODO
> list.
> 
> -patch 1 removes three items from the TODO file that were previously
>  completed-- multiple root dprc support, MSI support, and command
> serialization
> 
> -patch 2 makes some way overdue updates to README.txt from review
> comments on the mailing list last fall
> 
> -patches 3-5 update the binary interface for several objects to  sync with 
> latest
> MC firmware (todo item: "MC firmware uprev")
> 
> -patches 6-13 are cleanup items
>-change in how object versions are used in binding decisions
>-setting coherent dma_ops for devices if necessary
>-setting cacheable flag for object regions if applicable
>-getting the version of the root dprc from hardware, not based
> on what was in the .h file
>-dprc driver now refuses to probe if dprc version is not minimum
> expected
>-added quirk for bug in coherency flag for dpseci objects
>-allocator driver refuses to bind to dpmcp objects if version
> is not minimum expected
> 
> -patch 14 adds a second maintainer for the driver
> 
> Horia Geanta (1):
>   staging: fsl-mc: add quirk handling for dpseci objects < 4.0
> 
> Horia Geantă (1):
>   staging: fsl-mc: return -EINVAL for all fsl_mc_portal_allocate()
> failures
> 
> Itai Katz (5):
>   staging: fsl-mc: don't use object versions to make binding decisions
>   staging: fsl-mc: set cacheable flag for added devices if applicable
>   staging: fsl-mc: get version of root dprc from MC hardware
>   staging: fsl-mc: add dprc version check
>   staging: fsl-mc: add dpmcp version check
> 
> Stuart Yoder (7):
>   staging: fsl-mc: TODO updates
>   staging: fsl-mc: DPAA2 overview readme update
>   staging: fsl-mc: update dpmcp binary interface to v3.0
>   staging: fsl-mc: update dpbp binary interface to v2.2
>   staging: fsl-mc: update dprc binary interface to v5.1
>   staging: fsl-mc: set up coherent dma ops for added devices
>   MAINTAINERS: fsl-mc: Add second maintainer
> 
>  MAINTAINERS |1 +
>  drivers/staging/fsl-mc/README.txt   |  138 
> ---
>  drivers/staging/fsl-mc/TODO |   13 ---
>  drivers/staging/fsl-mc/bus/dpbp.c   |   77 ++-
>  drivers/staging/fsl-mc/bus/dpmcp-cmd.h  |7 +-
>  drivers/staging/fsl-mc/bus/dpmcp.c  |   35 +--
>  drivers/staging/fsl-mc/bus/dpmcp.h  |   10 +-
>  drivers/staging/fsl-mc/bus/dprc-cmd.h   |6 +-
>  drivers/staging/fsl-mc/bus/dprc-driver.c|   33 ++-
>  drivers/staging/fsl-mc/bus/dprc.c   |   26 ++---
>  drivers/staging/fsl-mc/bus/mc-allocator.c   |   18 ++--
>  drivers/staging/fsl-mc/bus/mc-bus.c |   93 +-
>  drivers/staging/fsl-mc/bus/mc-msi.c |2 +-
>  drivers/staging/fsl-mc/include/dpbp-cmd.h   |4 +-
>  drivers/staging/fsl-mc/include/dpbp.h   |   51 +-
>  drivers/staging/fsl-mc/include/dprc.h   |   19 ++--
>  drivers/staging/fsl-mc/include/mc-private.h |2 +
>  17 files changed, 335 insertions(+), 200 deletions(-)
> 
> --
> 1.7.9.5

Acked-by: German Rivera 

RE: linux-next: manual merge of the staging tree with the tip tree

[Jose Rivera]

> -Original Message-
> From: Greg KH [mailto:g...@kroah.com]
> Sent: Wednesday, March 02, 2016 11:44 AM
> To: Thomas Gleixner 
> Cc: Stephen Rothwell ; Ingo Molnar
> ; H. Peter Anvin ; Peter Zijlstra
> ; linux-n...@vger.kernel.org; linux-
> ker...@vger.kernel.org; J. German Rivera ;
> Qais Yousef 
> Subject: Re: linux-next: manual merge of the staging tree with the tip tree
> 
> On Wed, Mar 02, 2016 at 08:54:39AM +0100, Thomas Gleixner wrote:
> > On Wed, 2 Mar 2016, Stephen Rothwell wrote:
> > > Hi Greg,
> > >
> > > Today's linux-next merge of the staging tree got a conflict in:
> > >
> > >   include/linux/irqdomain.h
> > >
> > > between commit:
> > >
> > >   29d5c8db26ad ("genirq: Add DOMAIN_BUS_IPI")
> > >
> > > from the tip tree and commit:
> > >
> > >   9b1b282ccd81 ("irqdomain: Added domain bus token
> > > DOMAIN_BUS_FSL_MC_MSI")
> > >
> > > from the staging tree.
> > >
> > > I fixed it up (see below) and can carry the fix as necessary (no
> > > action is required).
> > >
> > > --
> > > Cheers,
> > > Stephen Rothwell
> > >
> > > diff --cc include/linux/irqdomain.h
> > > index ed48594e96d2,0934d06c8b00..
> > > --- a/include/linux/irqdomain.h
> > > +++ b/include/linux/irqdomain.h
> > > @@@ -74,7 -74,7 +74,8 @@@ enum irq_domain_bus_token
> > >   DOMAIN_BUS_PCI_MSI,
> > >   DOMAIN_BUS_PLATFORM_MSI,
> > >   DOMAIN_BUS_NEXUS,
> > >  +DOMAIN_BUS_IPI,
> > > + DOMAIN_BUS_FSL_MC_MSI,
> >
> > Errm. What's that? Did that DOMAIN_BUS_FSL_MC_MSI thingy get any
> > review of the irq[domain] maintainers?
> 
> For some reason I thought it did, but in looking at the patch series, it 
> doesn't
> seem like that happened :(
> 
> The patch series started with this message id on lkml:
>   Message-ID: <1452117809-1870-1-git-send-email-
> german.riv...@freescale.com>
> 
> German, any reason why you didn't cc: the irqdomain people on this series?
> 

This series was reviewed by Jiang Liu 
and Marc Zyngier . I apologize if I
overlooked to CC Thomas as well.

Thanks,

German

> Thomas, should I revert the series until you have a chance to ack it?
> 
> thanks,
> 
> greg k-h

RE: linux-next: manual merge of the staging tree with the tip tree

[Jose Rivera]

> -Original Message-
> From: Greg KH [mailto:g...@kroah.com]
> Sent: Wednesday, March 02, 2016 11:44 AM
> To: Thomas Gleixner 
> Cc: Stephen Rothwell ; Ingo Molnar
> ; H. Peter Anvin ; Peter Zijlstra
> ; linux-n...@vger.kernel.org; linux-
> ker...@vger.kernel.org; J. German Rivera ;
> Qais Yousef 
> Subject: Re: linux-next: manual merge of the staging tree with the tip tree
> 
> On Wed, Mar 02, 2016 at 08:54:39AM +0100, Thomas Gleixner wrote:
> > On Wed, 2 Mar 2016, Stephen Rothwell wrote:
> > > Hi Greg,
> > >
> > > Today's linux-next merge of the staging tree got a conflict in:
> > >
> > >   include/linux/irqdomain.h
> > >
> > > between commit:
> > >
> > >   29d5c8db26ad ("genirq: Add DOMAIN_BUS_IPI")
> > >
> > > from the tip tree and commit:
> > >
> > >   9b1b282ccd81 ("irqdomain: Added domain bus token
> > > DOMAIN_BUS_FSL_MC_MSI")
> > >
> > > from the staging tree.
> > >
> > > I fixed it up (see below) and can carry the fix as necessary (no
> > > action is required).
> > >
> > > --
> > > Cheers,
> > > Stephen Rothwell
> > >
> > > diff --cc include/linux/irqdomain.h
> > > index ed48594e96d2,0934d06c8b00..
> > > --- a/include/linux/irqdomain.h
> > > +++ b/include/linux/irqdomain.h
> > > @@@ -74,7 -74,7 +74,8 @@@ enum irq_domain_bus_token
> > >   DOMAIN_BUS_PCI_MSI,
> > >   DOMAIN_BUS_PLATFORM_MSI,
> > >   DOMAIN_BUS_NEXUS,
> > >  +DOMAIN_BUS_IPI,
> > > + DOMAIN_BUS_FSL_MC_MSI,
> >
> > Errm. What's that? Did that DOMAIN_BUS_FSL_MC_MSI thingy get any
> > review of the irq[domain] maintainers?
> 
> For some reason I thought it did, but in looking at the patch series, it 
> doesn't
> seem like that happened :(
> 
> The patch series started with this message id on lkml:
>   Message-ID: <1452117809-1870-1-git-send-email-
> german.riv...@freescale.com>
> 
> German, any reason why you didn't cc: the irqdomain people on this series?
> 

This series was reviewed by Jiang Liu 
and Marc Zyngier . I apologize if I
overlooked to CC Thomas as well.

Thanks,

German

> Thomas, should I revert the series until you have a chance to ack it?
> 
> thanks,
> 
> greg k-h

RE: [PATCH v2 03/11] staging: fsl-mc: Added generic MSI support for FSL-MC devices

[Jose Rivera]

> -Original Message-
> From: Marc Zyngier [mailto:marc.zyng...@arm.com]
> Sent: Monday, November 09, 2015 4:46 AM
> To: Rivera Jose-B46482; gre...@linuxfoundation.org; a...@arndb.de;
> de...@driverdev.osuosl.org; linux-kernel@vger.kernel.org
> Cc: Yoder Stuart-B08248; katz Itai-RM05202; Pan Lijun-B44306; Li Yang-
> Leo-R58472; Wood Scott-B07421; ag...@suse.de; Hamciuc Bogdan-BHAMCIU1;
> Marginean Alexandru-R89243; Sharma Bhupesh-B45370; Erez Nir-RM30794;
> Schmitt Richard-B43082; dan.carpen...@oracle.com;
> jiang@linux.intel.com
> Subject: Re: [PATCH v2 03/11] staging: fsl-mc: Added generic MSI support
> for FSL-MC devices
> 
> On 06/11/15 23:20, Jose Rivera wrote:
> 
> [...]
> 
> >>> +static void __fsl_mc_msi_write_msg(struct fsl_mc_device *mc_bus_dev,
> >>> +struct fsl_mc_device_irq *mc_dev_irq) {
> >>> + int error;
> >>> + struct fsl_mc_device *owner_mc_dev = mc_dev_irq->mc_dev;
> >>> + struct msi_desc *msi_desc = mc_dev_irq->msi_desc;
> >>> + struct dprc_irq_cfg irq_cfg;
> 
> [...]
> 
> >>> + /*
> >>> +  * DPRCs and other objects use different commands to set up IRQs,
> >>> +  * so we have to differentiate them here.
> >>> +  */
> >>> + if (owner_mc_dev == mc_bus_dev) {
> >>> + /*
> >>> +  * IRQ is for the mc_bus_dev's DPRC itself
> >>> +  */
> >>> + error = dprc_set_irq(mc_bus_dev->mc_io,
> >>> +  MC_CMD_FLAG_INTR_DIS | MC_CMD_FLAG_PRI,
> >>> +  mc_bus_dev->mc_handle,
> >>> +  mc_dev_irq->dev_irq_index,
> >>> +  _cfg);
> >>> + if (error < 0) {
> >>> + dev_err(_mc_dev->dev,
> >>> + "dprc_set_irq() failed: %d\n", error);
> >>> + }
> >>> + } else {
> >>> + error = dprc_set_obj_irq(mc_bus_dev->mc_io,
> >>> +  MC_CMD_FLAG_INTR_DIS | MC_CMD_FLAG_PRI,
> >>> +  mc_bus_dev->mc_handle,
> >>> +  owner_mc_dev->obj_desc.type,
> >>> +  owner_mc_dev->obj_desc.id,
> >>> +  mc_dev_irq->dev_irq_index,
> >>> +  _cfg);
> >>> + if (error < 0) {
> >>> + dev_err(_mc_dev->dev,
> >>> + "dprc_obj_set_irq() failed: %d\n", error);
> >>> + }
> >>> + }
> >>
> >> It feels a bit odd that you have all of this under a single MSI
> >> umbrella, and yet have to differentiate between them. Do they have a
> >> different programming interface? Or is that because these
> >> dprc_set_*_irq functions do some other stuff behind the scene (I'm too
> lazy to check...)?
> >>
> > Due to MC firmware API limitations, dprc_set_obj_irq can only be used
> > to set IRQs for the DPRC's children not for the DPRC itself.
> 
> Right. So this makes me wonder whether or not you have the right approach
> here. The logic behind the bus type was that devices with a common
> programming interface would share a bus type (the odd duck being platform
> which is used to implement anything else).
> 
> Your description seems to suggest that only devices behind the DPRC share
> that programming interface, and that the DPRC itself is the local weirdo.
> Should it be using the platform-msi subsystem instead? Or is it just a
> matter of firmware oddity?
> 
It's really a MC API oddity--  both the DPRC and the devices behind it share
the same MSI context (same ITT in the ITS) but they just require different
APIs to set the MSI addr/data.

> This is probably not a big deal, but it is worth keeping it in mind,
> specially if that has visible consequences (in your device tree, for
> example).
> 
> Thanks,
> 
>   M.
> --
> Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH v2 03/11] staging: fsl-mc: Added generic MSI support for FSL-MC devices

[Jose Rivera]

> -Original Message-
> From: Marc Zyngier [mailto:marc.zyng...@arm.com]
> Sent: Monday, November 09, 2015 4:46 AM
> To: Rivera Jose-B46482; gre...@linuxfoundation.org; a...@arndb.de;
> de...@driverdev.osuosl.org; linux-kernel@vger.kernel.org
> Cc: Yoder Stuart-B08248; katz Itai-RM05202; Pan Lijun-B44306; Li Yang-
> Leo-R58472; Wood Scott-B07421; ag...@suse.de; Hamciuc Bogdan-BHAMCIU1;
> Marginean Alexandru-R89243; Sharma Bhupesh-B45370; Erez Nir-RM30794;
> Schmitt Richard-B43082; dan.carpen...@oracle.com;
> jiang@linux.intel.com
> Subject: Re: [PATCH v2 03/11] staging: fsl-mc: Added generic MSI support
> for FSL-MC devices
> 
> On 06/11/15 23:20, Jose Rivera wrote:
> 
> [...]
> 
> >>> +static void __fsl_mc_msi_write_msg(struct fsl_mc_device *mc_bus_dev,
> >>> +struct fsl_mc_device_irq *mc_dev_irq) {
> >>> + int error;
> >>> + struct fsl_mc_device *owner_mc_dev = mc_dev_irq->mc_dev;
> >>> + struct msi_desc *msi_desc = mc_dev_irq->msi_desc;
> >>> + struct dprc_irq_cfg irq_cfg;
> 
> [...]
> 
> >>> + /*
> >>> +  * DPRCs and other objects use different commands to set up IRQs,
> >>> +  * so we have to differentiate them here.
> >>> +  */
> >>> + if (owner_mc_dev == mc_bus_dev) {
> >>> + /*
> >>> +  * IRQ is for the mc_bus_dev's DPRC itself
> >>> +  */
> >>> + error = dprc_set_irq(mc_bus_dev->mc_io,
> >>> +  MC_CMD_FLAG_INTR_DIS | MC_CMD_FLAG_PRI,
> >>> +  mc_bus_dev->mc_handle,
> >>> +  mc_dev_irq->dev_irq_index,
> >>> +  _cfg);
> >>> + if (error < 0) {
> >>> + dev_err(_mc_dev->dev,
> >>> + "dprc_set_irq() failed: %d\n", error);
> >>> + }
> >>> + } else {
> >>> + error = dprc_set_obj_irq(mc_bus_dev->mc_io,
> >>> +  MC_CMD_FLAG_INTR_DIS | MC_CMD_FLAG_PRI,
> >>> +  mc_bus_dev->mc_handle,
> >>> +  owner_mc_dev->obj_desc.type,
> >>> +  owner_mc_dev->obj_desc.id,
> >>> +  mc_dev_irq->dev_irq_index,
> >>> +  _cfg);
> >>> + if (error < 0) {
> >>> + dev_err(_mc_dev->dev,
> >>> + "dprc_obj_set_irq() failed: %d\n", error);
> >>> + }
> >>> + }
> >>
> >> It feels a bit odd that you have all of this under a single MSI
> >> umbrella, and yet have to differentiate between them. Do they have a
> >> different programming interface? Or is that because these
> >> dprc_set_*_irq functions do some other stuff behind the scene (I'm too
> lazy to check...)?
> >>
> > Due to MC firmware API limitations, dprc_set_obj_irq can only be used
> > to set IRQs for the DPRC's children not for the DPRC itself.
> 
> Right. So this makes me wonder whether or not you have the right approach
> here. The logic behind the bus type was that devices with a common
> programming interface would share a bus type (the odd duck being platform
> which is used to implement anything else).
> 
> Your description seems to suggest that only devices behind the DPRC share
> that programming interface, and that the DPRC itself is the local weirdo.
> Should it be using the platform-msi subsystem instead? Or is it just a
> matter of firmware oddity?
> 
It's really a MC API oddity--  both the DPRC and the devices behind it share
the same MSI context (same ITT in the ITS) but they just require different
APIs to set the MSI addr/data.

> This is probably not a big deal, but it is worth keeping it in mind,
> specially if that has visible consequences (in your device tree, for
> example).
> 
> Thanks,
> 
>   M.
> --
> Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH v2 03/11] staging: fsl-mc: Added generic MSI support for FSL-MC devices

[Jose Rivera]

> -Original Message-
> From: Marc Zyngier [mailto:marc.zyng...@arm.com]
> Sent: Friday, November 06, 2015 11:51 AM
> To: Rivera Jose-B46482; gre...@linuxfoundation.org; a...@arndb.de;
> de...@driverdev.osuosl.org; linux-kernel@vger.kernel.org
> Cc: Yoder Stuart-B08248; katz Itai-RM05202; Pan Lijun-B44306; Li Yang-
> Leo-R58472; Wood Scott-B07421; ag...@suse.de; Hamciuc Bogdan-BHAMCIU1;
> Marginean Alexandru-R89243; Sharma Bhupesh-B45370; Erez Nir-RM30794;
> Schmitt Richard-B43082; dan.carpen...@oracle.com;
> jiang@linux.intel.com
> Subject: Re: [PATCH v2 03/11] staging: fsl-mc: Added generic MSI support
> for FSL-MC devices
> 
> On 30/10/15 19:43, J. German Rivera wrote:
> > Created an MSI domain for the fsl-mc bus-- including functions to
> > create a domain, find a domain, alloc/free domain irqs, and bus
> > specific overrides for domain and irq_chip ops.
> >
> > Signed-off-by: J. German Rivera 
> > ---
> > Changes in v2: none
> >
> >  drivers/staging/fsl-mc/bus/Kconfig  |   1 +
> >  drivers/staging/fsl-mc/bus/Makefile |   1 +
> >  drivers/staging/fsl-mc/bus/mc-msi.c | 276
> 
> >  drivers/staging/fsl-mc/include/mc-private.h |  17 ++
> >  drivers/staging/fsl-mc/include/mc.h |  17 ++
> >  5 files changed, 312 insertions(+)
> >  create mode 100644 drivers/staging/fsl-mc/bus/mc-msi.c
> >
> > diff --git a/drivers/staging/fsl-mc/bus/Kconfig
> > b/drivers/staging/fsl-mc/bus/Kconfig
> > index 0d779d9..c498ac6 100644
> > --- a/drivers/staging/fsl-mc/bus/Kconfig
> > +++ b/drivers/staging/fsl-mc/bus/Kconfig
> > @@ -9,6 +9,7 @@
> >  config FSL_MC_BUS
> > tristate "Freescale Management Complex (MC) bus driver"
> > depends on OF && ARM64
> > +   select GENERIC_MSI_IRQ_DOMAIN
> > help
> >   Driver to enable the bus infrastructure for the Freescale
> >QorIQ Management Complex (fsl-mc). The fsl-mc is a hardware
> > diff --git a/drivers/staging/fsl-mc/bus/Makefile
> > b/drivers/staging/fsl-mc/bus/Makefile
> > index 25433a9..a5f2ba4 100644
> > --- a/drivers/staging/fsl-mc/bus/Makefile
> > +++ b/drivers/staging/fsl-mc/bus/Makefile
> > @@ -13,5 +13,6 @@ mc-bus-driver-objs := mc-bus.o \
> >   dpmng.o \
> >   dprc-driver.o \
> >   mc-allocator.o \
> > + mc-msi.o \
> >   dpmcp.o \
> >   dpbp.o
> > diff --git a/drivers/staging/fsl-mc/bus/mc-msi.c
> > b/drivers/staging/fsl-mc/bus/mc-msi.c
> > new file mode 100644
> > index 000..81b53e3
> > --- /dev/null
> > +++ b/drivers/staging/fsl-mc/bus/mc-msi.c
> > @@ -0,0 +1,276 @@
> > +/*
> > + * Freescale Management Complex (MC) bus driver MSI support
> > + *
> > + * Copyright (C) 2015 Freescale Semiconductor, Inc.
> > + * Author: German Rivera 
> > + *
> > + * This file is licensed under the terms of the GNU General Public
> > + * License version 2. This program is licensed "as is" without any
> > + * warranty of any kind, whether express or implied.
> > + */
> > +
> > +#include "../include/mc-private.h"
> > +#include 
> > +#include 
> > +#include  #include  #include
> > + #include  #include
> > +"../include/mc-sys.h"
> > +#include "dprc-cmd.h"
> > +
> > +static void fsl_mc_msi_set_desc(msi_alloc_info_t *arg,
> > +   struct msi_desc *desc)
> > +{
> > +   arg->desc = desc;
> > +   arg->hwirq = (irq_hw_number_t)desc->fsl_mc.msi_index;
> > +}
> > +
> > +static void fsl_mc_msi_update_dom_ops(struct msi_domain_info *info) {
> > +   struct msi_domain_ops *ops = info->ops;
> > +
> > +   if (WARN_ON(!ops))
> > +   return;
> > +
> > +   if (!ops->set_desc)
> > +   ops->set_desc = fsl_mc_msi_set_desc;
> 
> When would that be overridden by the MSI controller?
>
It should not be set by the MSI controller. As you suggested in another, I will 
add
A WARN_ON if set_desc is not NULL.
 
> > +}
> > +
> > +static void __fsl_mc_msi_write_msg(struct fsl_mc_device *mc_bus_dev,
> > +  struct fsl_mc_device_irq *mc_dev_irq) {
> > +   int error;
> > +   struct fsl_mc_device *owner_mc_dev = mc_dev_irq->mc_dev;
> > +   struct msi_desc *msi_desc = mc_dev_irq->msi_desc;
> > +   struct dprc_irq_cfg irq_cfg;
> > +
> > +   /*
> > +* msi_desc->msg.address is 0x0 when this function is invoked in
> > +* the free_irq() code path. In this case, for the MC, we don't
> > +* really need to "unprogram" the MSI, so we just return.
> > +*/
> > +   if (msi_desc->msg.address_lo == 0x0 && msi_desc->msg.address_hi ==
> 0x0)
> > +   return;
> > +
> > +   if (WARN_ON(!owner_mc_dev))
> > +   return;
> > +
> > +   irq_cfg.paddr = ((u64)msi_desc->msg.address_hi << 32) |
> > +   msi_desc->msg.address_lo;
> 
> This should really be a phys_addr_t.
> 
I'll change the type of paddr in the dprc_irq_cfg struct to be phys_addr_t 
instead of u64.

> > +   irq_cfg.val = msi_desc->msg.data;
> > +   irq_cfg.user_irq_id = msi_desc->irq;
> > +

RE: [PATCH] arm64: dts: Added syscon-reboot node for FSL's LS2080A SoC

[Jose Rivera]

> -Original Message-
> From: Javier Martinez Canillas [mailto:jav...@dowhile0.org]
> Sent: Friday, November 06, 2015 4:13 PM
> To: Rivera Jose-B46482
> Cc: Rob Herring; Mark Rutland; devicet...@vger.kernel.org; linux-arm-
> ker...@lists.infradead.org; Linux Kernel; Sharma Bhupesh-B45370; Li Yang-
> Leo-R58472; Yoder Stuart-B08248
> Subject: Re: [PATCH] arm64: dts: Added syscon-reboot node for FSL's
> LS2080A SoC
> 
> Hello J. German,
> 
> On Fri, Nov 6, 2015 at 6:48 PM, J. German Rivera
>  wrote:
> > Added sys-reboot node to the FSL's LS2080A SoC DT to leverage the
> > ARM-generic reboot mechanism for this SoC. This mechanism is enabled
> > through CONFIG_POWER_RESET_SYSCON.
> >
> 
> What's in the changelog ends as a part of the commit message and usually
> the change history is not added there...
> 
You are right, thanks. I forgot to move the CHANGE HISTORY to be below the "---"
Marker. I'll repost the patch.

> > CHANGE HISTORY:
> >
> > Changes in v3:
> > - Addressed comment from Stuart Yoder
> >   * Fixed commit message to refer to LS2080A instead of LS2085A.
> >
> > Changes in v2:
> > - Addressed comment form Stuart Yoder:
> >   * Removed "@" from reboot node
> >
> > Changes in v3:
> > - Addressed comment form Stuart Yoder:
> >   * Expose only the reset register
> >
> > Changes in v4:
> > - Addressed comment from Arnd Bergmann:
> >   * Changed compatible string to be more specific
> >   * Changed node name to 'syscon' instead of 'rstcr'
> >   * Changed address to lower case hex
> > - Addressed comment form Stuart Yoder:
> >   * Rebase on top of branch arm-soc/for-next
> >
> > Signed-off-by: J. German Rivera 
> > ---
> 
> ...so the change history should be added here, between the '---'
> marker line and the patch diff since that section is omitted by tools
> like git-am when applying patches.
> 
> >  arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi | 12 
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
> > b/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
> > index e81cd48..a790a90 100644
> > --- a/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
> > @@ -153,6 +153,18 @@
> > };
> > };
> >
> > +   rstcr: syscon@1e6 {
> > +   compatible = "fsl,ls2085a-rstcr", "syscon";
> > +   reg = <0x0 0x1e6 0x0 0x4>;
> > +   };
> > +
> > +   reboot {
> > +   compatible ="syscon-reboot";
> > +   regmap = <>;
> > +   offset = <0x0>;
> > +   mask = <0x2>;
> > +   };
> > +
> > timer {
> > compatible = "arm,armv8-timer";
> > interrupts = <1 13 0x8>, /* Physical Secure PPI,
> > active-low */
> > --
> 
> The patch looks good to me though.
> 
> Reviewed-by: Javier Martinez Canillas 
> 
> Best regards,
> Javier

RE: [PATCH v2 03/11] staging: fsl-mc: Added generic MSI support for FSL-MC devices

[Jose Rivera]

> -Original Message-
> From: Marc Zyngier [mailto:marc.zyng...@arm.com]
> Sent: Friday, November 06, 2015 11:51 AM
> To: Rivera Jose-B46482; gre...@linuxfoundation.org; a...@arndb.de;
> de...@driverdev.osuosl.org; linux-kernel@vger.kernel.org
> Cc: Yoder Stuart-B08248; katz Itai-RM05202; Pan Lijun-B44306; Li Yang-
> Leo-R58472; Wood Scott-B07421; ag...@suse.de; Hamciuc Bogdan-BHAMCIU1;
> Marginean Alexandru-R89243; Sharma Bhupesh-B45370; Erez Nir-RM30794;
> Schmitt Richard-B43082; dan.carpen...@oracle.com;
> jiang@linux.intel.com
> Subject: Re: [PATCH v2 03/11] staging: fsl-mc: Added generic MSI support
> for FSL-MC devices
> 
> On 30/10/15 19:43, J. German Rivera wrote:
> > Created an MSI domain for the fsl-mc bus-- including functions to
> > create a domain, find a domain, alloc/free domain irqs, and bus
> > specific overrides for domain and irq_chip ops.
> >
> > Signed-off-by: J. German Rivera 
> > ---
> > Changes in v2: none
> >
> >  drivers/staging/fsl-mc/bus/Kconfig  |   1 +
> >  drivers/staging/fsl-mc/bus/Makefile |   1 +
> >  drivers/staging/fsl-mc/bus/mc-msi.c | 276
> 
> >  drivers/staging/fsl-mc/include/mc-private.h |  17 ++
> >  drivers/staging/fsl-mc/include/mc.h |  17 ++
> >  5 files changed, 312 insertions(+)
> >  create mode 100644 drivers/staging/fsl-mc/bus/mc-msi.c
> >
> > diff --git a/drivers/staging/fsl-mc/bus/Kconfig
> > b/drivers/staging/fsl-mc/bus/Kconfig
> > index 0d779d9..c498ac6 100644
> > --- a/drivers/staging/fsl-mc/bus/Kconfig
> > +++ b/drivers/staging/fsl-mc/bus/Kconfig
> > @@ -9,6 +9,7 @@
> >  config FSL_MC_BUS
> > tristate "Freescale Management Complex (MC) bus driver"
> > depends on OF && ARM64
> > +   select GENERIC_MSI_IRQ_DOMAIN
> > help
> >   Driver to enable the bus infrastructure for the Freescale
> >QorIQ Management Complex (fsl-mc). The fsl-mc is a hardware
> > diff --git a/drivers/staging/fsl-mc/bus/Makefile
> > b/drivers/staging/fsl-mc/bus/Makefile
> > index 25433a9..a5f2ba4 100644
> > --- a/drivers/staging/fsl-mc/bus/Makefile
> > +++ b/drivers/staging/fsl-mc/bus/Makefile
> > @@ -13,5 +13,6 @@ mc-bus-driver-objs := mc-bus.o \
> >   dpmng.o \
> >   dprc-driver.o \
> >   mc-allocator.o \
> > + mc-msi.o \
> >   dpmcp.o \
> >   dpbp.o
> > diff --git a/drivers/staging/fsl-mc/bus/mc-msi.c
> > b/drivers/staging/fsl-mc/bus/mc-msi.c
> > new file mode 100644
> > index 000..81b53e3
> > --- /dev/null
> > +++ b/drivers/staging/fsl-mc/bus/mc-msi.c
> > @@ -0,0 +1,276 @@
> > +/*
> > + * Freescale Management Complex (MC) bus driver MSI support
> > + *
> > + * Copyright (C) 2015 Freescale Semiconductor, Inc.
> > + * Author: German Rivera 
> > + *
> > + * This file is licensed under the terms of the GNU General Public
> > + * License version 2. This program is licensed "as is" without any
> > + * warranty of any kind, whether express or implied.
> > + */
> > +
> > +#include "../include/mc-private.h"
> > +#include 
> > +#include 
> > +#include  #include  #include
> > + #include  #include
> > +"../include/mc-sys.h"
> > +#include "dprc-cmd.h"
> > +
> > +static void fsl_mc_msi_set_desc(msi_alloc_info_t *arg,
> > +   struct msi_desc *desc)
> > +{
> > +   arg->desc = desc;
> > +   arg->hwirq = (irq_hw_number_t)desc->fsl_mc.msi_index;
> > +}
> > +
> > +static void fsl_mc_msi_update_dom_ops(struct msi_domain_info *info) {
> > +   struct msi_domain_ops *ops = info->ops;
> > +
> > +   if (WARN_ON(!ops))
> > +   return;
> > +
> > +   if (!ops->set_desc)
> > +   ops->set_desc = fsl_mc_msi_set_desc;
> 
> When would that be overridden by the MSI controller?
>
It should not be set by the MSI controller. As you suggested in another, I will 
add
A WARN_ON if set_desc is not NULL.
 
> > +}
> > +
> > +static void __fsl_mc_msi_write_msg(struct fsl_mc_device *mc_bus_dev,
> > +  struct fsl_mc_device_irq *mc_dev_irq) {
> > +   int error;
> > +   struct fsl_mc_device *owner_mc_dev = mc_dev_irq->mc_dev;
> > +   struct msi_desc *msi_desc = mc_dev_irq->msi_desc;
> > +   struct dprc_irq_cfg irq_cfg;
> > +
> > +   /*
> > +* msi_desc->msg.address is 0x0 when this function is invoked in
> > +* the free_irq() code path. In this case, for the MC, we don't
> > +* really need to "unprogram" the MSI, so we just return.
> > +*/
> > +   if (msi_desc->msg.address_lo == 0x0 && msi_desc->msg.address_hi ==
> 0x0)
> > +   return;
> > +
> > +   if (WARN_ON(!owner_mc_dev))
> > +   return;
> > +
> > +   irq_cfg.paddr = ((u64)msi_desc->msg.address_hi << 32) |
> > +   msi_desc->msg.address_lo;
> 
> This should really be a phys_addr_t.
> 
I'll change the type of paddr in the dprc_irq_cfg struct to be phys_addr_t 
instead of u64.

> > +   irq_cfg.val = 

RE: [PATCH] arm64: dts: Added syscon-reboot node for FSL's LS2080A SoC

[Jose Rivera]

> -Original Message-
> From: Javier Martinez Canillas [mailto:jav...@dowhile0.org]
> Sent: Friday, November 06, 2015 4:13 PM
> To: Rivera Jose-B46482
> Cc: Rob Herring; Mark Rutland; devicet...@vger.kernel.org; linux-arm-
> ker...@lists.infradead.org; Linux Kernel; Sharma Bhupesh-B45370; Li Yang-
> Leo-R58472; Yoder Stuart-B08248
> Subject: Re: [PATCH] arm64: dts: Added syscon-reboot node for FSL's
> LS2080A SoC
> 
> Hello J. German,
> 
> On Fri, Nov 6, 2015 at 6:48 PM, J. German Rivera
>  wrote:
> > Added sys-reboot node to the FSL's LS2080A SoC DT to leverage the
> > ARM-generic reboot mechanism for this SoC. This mechanism is enabled
> > through CONFIG_POWER_RESET_SYSCON.
> >
> 
> What's in the changelog ends as a part of the commit message and usually
> the change history is not added there...
> 
You are right, thanks. I forgot to move the CHANGE HISTORY to be below the "---"
Marker. I'll repost the patch.

> > CHANGE HISTORY:
> >
> > Changes in v3:
> > - Addressed comment from Stuart Yoder
> >   * Fixed commit message to refer to LS2080A instead of LS2085A.
> >
> > Changes in v2:
> > - Addressed comment form Stuart Yoder:
> >   * Removed "@" from reboot node
> >
> > Changes in v3:
> > - Addressed comment form Stuart Yoder:
> >   * Expose only the reset register
> >
> > Changes in v4:
> > - Addressed comment from Arnd Bergmann:
> >   * Changed compatible string to be more specific
> >   * Changed node name to 'syscon' instead of 'rstcr'
> >   * Changed address to lower case hex
> > - Addressed comment form Stuart Yoder:
> >   * Rebase on top of branch arm-soc/for-next
> >
> > Signed-off-by: J. German Rivera 
> > ---
> 
> ...so the change history should be added here, between the '---'
> marker line and the patch diff since that section is omitted by tools
> like git-am when applying patches.
> 
> >  arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi | 12 
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
> > b/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
> > index e81cd48..a790a90 100644
> > --- a/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
> > @@ -153,6 +153,18 @@
> > };
> > };
> >
> > +   rstcr: syscon@1e6 {
> > +   compatible = "fsl,ls2085a-rstcr", "syscon";
> > +   reg = <0x0 0x1e6 0x0 0x4>;
> > +   };
> > +
> > +   reboot {
> > +   compatible ="syscon-reboot";
> > +   regmap = <>;
> > +   offset = <0x0>;
> > +   mask = <0x2>;
> > +   };
> > +
> > timer {
> > compatible = "arm,armv8-timer";
> > interrupts = <1 13 0x8>, /* Physical Secure PPI,
> > active-low */
> > --
> 
> The patch looks good to me though.
> 
> Reviewed-by: Javier Martinez Canillas 
> 
> Best regards,
> Javier

RE: [PATCH 0/4] staging: fsl-mc: MC command serialization

[Jose Rivera]

> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Saturday, October 17, 2015 1:16 AM
> To: Rivera Jose-B46482
> Cc: a...@arndb.de; de...@driverdev.osuosl.org; linux-
> ker...@vger.kernel.org; Hamciuc Bogdan-BHAMCIU1; Sharma Bhupesh-B45370;
> ag...@suse.de; Yoder Stuart-B08248; Erez Nir-RM30794; katz Itai-RM05202;
> Wood Scott-B07421; Pan Lijun-B44306; Li Yang-Leo-R58472; Marginean
> Alexandru-R89243; dan.carpen...@oracle.com; Schmitt Richard-B43082
> Subject: Re: [PATCH 0/4] staging: fsl-mc: MC command serialization
> 
> On Wed, Oct 14, 2015 at 03:11:41PM -0500, J. German Rivera wrote:
> > This patch series depends on the patch series posted at
> > http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg996780.htm
> > l
> 
> I didn't take that series for obvious reasons :(

Greg,

Do I need to respin this series to include the updated link to the patch
series this series depends on
(http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg999081.html)?

Thanks,

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

RE: [PATCH v2 01/12] staging: fsl-mc: Naming cleanup in fsl_mc-portal_allocate

[Jose Rivera]

> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Saturday, October 17, 2015 1:16 AM
> To: Rivera Jose-B46482
> Cc: a...@arndb.de; de...@driverdev.osuosl.org; linux-
> ker...@vger.kernel.org; Hamciuc Bogdan-BHAMCIU1; Sharma Bhupesh-B45370;
> ag...@suse.de; Yoder Stuart-B08248; Erez Nir-RM30794; katz Itai-RM05202;
> Wood Scott-B07421; Pan Lijun-B44306; Li Yang-Leo-R58472; Marginean
> Alexandru-R89243; dan.carpen...@oracle.com; Schmitt Richard-B43082
> Subject: Re: [PATCH v2 01/12] staging: fsl-mc: Naming cleanup in fsl_mc-
> portal_allocate
> 
> On Wed, Oct 14, 2015 at 02:51:40PM -0500, J. German Rivera wrote:
> > mc_adev is a local variable for the allocated dpmcp object.
> > Renamed mc_adev as dpmcp_dev for clarity.
> > ---
> > CHANGE HISTORY
> >
> > Changes in v2: none
> 
> None of the patches in this series has a signed-off-by: line, as required
> by Documentation/SubmittingPatches, so I'm guessing you don't want them
> actually applied :(

Greg,

I apologize for that oversight. I'll send a respin fixing this.

Thanks,

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

RE: [PATCH 0/4] staging: fsl-mc: MC command serialization

[Jose Rivera]

> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Saturday, October 17, 2015 1:16 AM
> To: Rivera Jose-B46482
> Cc: a...@arndb.de; de...@driverdev.osuosl.org; linux-
> ker...@vger.kernel.org; Hamciuc Bogdan-BHAMCIU1; Sharma Bhupesh-B45370;
> ag...@suse.de; Yoder Stuart-B08248; Erez Nir-RM30794; katz Itai-RM05202;
> Wood Scott-B07421; Pan Lijun-B44306; Li Yang-Leo-R58472; Marginean
> Alexandru-R89243; dan.carpen...@oracle.com; Schmitt Richard-B43082
> Subject: Re: [PATCH 0/4] staging: fsl-mc: MC command serialization
> 
> On Wed, Oct 14, 2015 at 03:11:41PM -0500, J. German Rivera wrote:
> > This patch series depends on the patch series posted at
> > http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg996780.htm
> > l
> 
> I didn't take that series for obvious reasons :(

Greg,

Do I need to respin this series to include the updated link to the patch
series this series depends on
(http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg999081.html)?

Thanks,

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

RE: [PATCH v2 01/12] staging: fsl-mc: Naming cleanup in fsl_mc-portal_allocate

[Jose Rivera]

> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Saturday, October 17, 2015 1:16 AM
> To: Rivera Jose-B46482
> Cc: a...@arndb.de; de...@driverdev.osuosl.org; linux-
> ker...@vger.kernel.org; Hamciuc Bogdan-BHAMCIU1; Sharma Bhupesh-B45370;
> ag...@suse.de; Yoder Stuart-B08248; Erez Nir-RM30794; katz Itai-RM05202;
> Wood Scott-B07421; Pan Lijun-B44306; Li Yang-Leo-R58472; Marginean
> Alexandru-R89243; dan.carpen...@oracle.com; Schmitt Richard-B43082
> Subject: Re: [PATCH v2 01/12] staging: fsl-mc: Naming cleanup in fsl_mc-
> portal_allocate
> 
> On Wed, Oct 14, 2015 at 02:51:40PM -0500, J. German Rivera wrote:
> > mc_adev is a local variable for the allocated dpmcp object.
> > Renamed mc_adev as dpmcp_dev for clarity.
> > ---
> > CHANGE HISTORY
> >
> > Changes in v2: none
> 
> None of the patches in this series has a signed-off-by: line, as required
> by Documentation/SubmittingPatches, so I'm guessing you don't want them
> actually applied :(

Greg,

I apologize for that oversight. I'll send a respin fixing this.

Thanks,

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

RE: [PATCH 2/3] Docs: dt: fsl-mc update binding to include definition of ranges

[Jose Rivera]

> -Original Message-
> From: Stuart Yoder [mailto:stuart.yo...@freescale.com]
> Sent: Friday, September 25, 2015 4:08 PM
> To: robh...@kernel.org; mark.rutl...@arm.com
> Cc: devicet...@vger.kernel.org; linux-kernel@vger.kernel.org; Rivera
> Jose-B46482; Sharma Bhupesh-B45370; katz Itai-RM05202; Yoder Stuart-
> B08248
> Subject: [PATCH 2/3] Docs: dt: fsl-mc update binding to include
> definition of ranges
> 
> Define a ranges property to specify the mapping between the MC address
> space and the system address space.
> 
> Signed-off-by: Stuart Yoder 
> ---
>  .../devicetree/bindings/misc/fsl,qoriq-mc.txt  | 30
> +-
>  1 file changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> index 6aac955..848975f 100644
> --- a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> +++ b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> @@ -35,6 +35,26 @@ Required properties:
>  Definition: Must be present and point to the MSI controller node
>  handling message interrupts for the MC.
> 
> +- ranges
> +Value type: 
> +Definition: A standard property.  Defines the mapping between
> the child
> +MC address space and the parent system address
> space.
> +
> +The MC address space is defined by 3 components:
> + 
> +
> +Valid values for region type are
> +   0x0 - MC portals
> +   0x1 - QBMAN portals
> +
> +- #address-cells
> +Value type: 
> +Definition: Must be 3.  (see definition in 'ranges' property)
> +
> +- #size-cells
> +Value type: 
> +Definition: Must be 1.
> +
>  Example:
> 
>  fsl_mc: fsl-mc@80c00 {
> @@ -42,5 +62,13 @@ Example:
>  reg = <0x0008 0x0c00 0 0x40>,/* MC portal
> base */
><0x 0x0834 0 0x4>; /* MC control
> reg */
>  msi-parent = <>;
> -};
> +#address-cells = <3>;
> +#size-cells = <1>;
> 
> +/*
> + * Region type 0x0 - MC portals
> + * Region type 0x1 - QBMAN portals
> + */
> +ranges = <0x0 0x0 0x0 0x8 0x0c00 0x400
> +  0x1 0x0 0x0 0x8 0x1800 0x800>;
> +};
> --
> 2.3.3

Acked-by: J. German Rivera 

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

RE: [PATCH 3/3] Docs: dt: fsl-mc: update binding to define dpmac subnodes

[Jose Rivera]

> -Original Message-
> From: Stuart Yoder [mailto:stuart.yo...@freescale.com]
> Sent: Friday, September 25, 2015 4:08 PM
> To: robh...@kernel.org; mark.rutl...@arm.com
> Cc: devicet...@vger.kernel.org; linux-kernel@vger.kernel.org; Rivera
> Jose-B46482; Sharma Bhupesh-B45370; katz Itai-RM05202; Yoder Stuart-
> B08248
> Subject: [PATCH 3/3] Docs: dt: fsl-mc: update binding to define dpmac
> subnodes
> 
> The fsl-mc node may optionally have dpmac sub-nodes that describe the
> relationship between the Ethernet MACs which belong to the MC and the
> Ethernet PHYs on the system board.
> 
> Signed-off-by: Stuart Yoder 
> ---
>  .../devicetree/bindings/misc/fsl,qoriq-mc.txt  | 45
> ++
>  1 file changed, 45 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> index 848975f..6611a7c 100644
> --- a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> +++ b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> @@ -55,6 +55,40 @@ Required properties:
>  Value type: 
>  Definition: Must be 1.
> 
> +Sub-nodes:
> +
> +The fsl-mc node may optionally have dpmac sub-nodes that
> describe
> +the relationship between the Ethernet MACs which belong to the
> MC
> +and the Ethernet PHYs on the system board.
> +
> +The dpmac nodes must be under a node named "dpmacs" which
> contains
> +the following properties:
> +
> +- #address-cells
> +  Value type: 
> +  Definition: Must be present if dpmac sub-nodes are defined
> and must
> +  have a value of 1.
> +
> +- #size-cells
> +  Value type: 
> +  Definition: Must be present if dpmac sub-nodes are defined
> and must
> +  have a value of 0.
> +
> +These nodes must have the following properties:
> +
> +- compatible
> +  Value type: 
> +  Definition: Must be "fsl,qoriq-mc-dpmac".
> +
> +- reg
> +  Value type: 
> +  Definition: Specifies the id of the dpmac.
> +
> +- phy-handle
> +  Value type: 
> +  Definition: Specifies the phandle to the PHY device node
> associated
> +  with the this dpmac.
> +
>  Example:
> 
>  fsl_mc: fsl-mc@80c00 {
> @@ -71,4 +105,15 @@ Example:
>   */
>  ranges = <0x0 0x0 0x0 0x8 0x0c00 0x400
>0x1 0x0 0x0 0x8 0x1800 0x800>;
> +
> +dpmacs {
> +#address-cells = <1>;
> +#size-cells = <0>;
> +
> +dpmac@1 {
> +compatible = "fsl,qoriq-mc-dpmac";
> +reg = <1>;
> +phy-handle = <_phy0>;
> +}
> +}
>  };
> --
> 2.3.3

Acked-by: J. German Rivera 

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

RE: [PATCH 1/3] Docs: dt: fsl-mc: update binding to include msi-parent

[Jose Rivera]

> -Original Message-
> From: Stuart Yoder [mailto:stuart.yo...@freescale.com]
> Sent: Friday, September 25, 2015 4:08 PM
> To: robh...@kernel.org; mark.rutl...@arm.com
> Cc: devicet...@vger.kernel.org; linux-kernel@vger.kernel.org; Rivera
> Jose-B46482; Sharma Bhupesh-B45370; katz Itai-RM05202; Yoder Stuart-
> B08248
> Subject: [PATCH 1/3] Docs: dt: fsl-mc: update binding to include msi-
> parent
> 
> The Freescale Management Complex and all associated objects use message
> interrupts, and thus an msi-parent is required.
> 
> Signed-off-by: Stuart Yoder 
> ---
>  Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> index c7a26ca..6aac955 100644
> --- a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> +++ b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> @@ -30,11 +30,17 @@ Required properties:
>  region may not be present in some scenarios,
> such
>  as in the device tree presented to a virtual
> machine.
> 
> +- msi-parent
> +Value type: 
> +Definition: Must be present and point to the MSI controller node
> +handling message interrupts for the MC.
> +
>  Example:
> 
>  fsl_mc: fsl-mc@80c00 {
>  compatible = "fsl,qoriq-mc";
>  reg = <0x0008 0x0c00 0 0x40>,/* MC portal
> base */
><0x 0x0834 0 0x4>; /* MC control
> reg */
> +msi-parent = <>;
>  };
> 
> --
> 2.3.3

Acked-by: J. German Rivera 

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

RE: [PATCH 1/3] Docs: dt: fsl-mc: update binding to include msi-parent

[Jose Rivera]

> -Original Message-
> From: Stuart Yoder [mailto:stuart.yo...@freescale.com]
> Sent: Friday, September 25, 2015 4:08 PM
> To: robh...@kernel.org; mark.rutl...@arm.com
> Cc: devicet...@vger.kernel.org; linux-kernel@vger.kernel.org; Rivera
> Jose-B46482; Sharma Bhupesh-B45370; katz Itai-RM05202; Yoder Stuart-
> B08248
> Subject: [PATCH 1/3] Docs: dt: fsl-mc: update binding to include msi-
> parent
> 
> The Freescale Management Complex and all associated objects use message
> interrupts, and thus an msi-parent is required.
> 
> Signed-off-by: Stuart Yoder 
> ---
>  Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> index c7a26ca..6aac955 100644
> --- a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> +++ b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> @@ -30,11 +30,17 @@ Required properties:
>  region may not be present in some scenarios,
> such
>  as in the device tree presented to a virtual
> machine.
> 
> +- msi-parent
> +Value type: 
> +Definition: Must be present and point to the MSI controller node
> +handling message interrupts for the MC.
> +
>  Example:
> 
>  fsl_mc: fsl-mc@80c00 {
>  compatible = "fsl,qoriq-mc";
>  reg = <0x0008 0x0c00 0 0x40>,/* MC portal
> base */
><0x 0x0834 0 0x4>; /* MC control
> reg */
> +msi-parent = <>;
>  };
> 
> --
> 2.3.3

Acked-by: J. German Rivera 

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

RE: [PATCH 3/3] Docs: dt: fsl-mc: update binding to define dpmac subnodes

[Jose Rivera]

> -Original Message-
> From: Stuart Yoder [mailto:stuart.yo...@freescale.com]
> Sent: Friday, September 25, 2015 4:08 PM
> To: robh...@kernel.org; mark.rutl...@arm.com
> Cc: devicet...@vger.kernel.org; linux-kernel@vger.kernel.org; Rivera
> Jose-B46482; Sharma Bhupesh-B45370; katz Itai-RM05202; Yoder Stuart-
> B08248
> Subject: [PATCH 3/3] Docs: dt: fsl-mc: update binding to define dpmac
> subnodes
> 
> The fsl-mc node may optionally have dpmac sub-nodes that describe the
> relationship between the Ethernet MACs which belong to the MC and the
> Ethernet PHYs on the system board.
> 
> Signed-off-by: Stuart Yoder 
> ---
>  .../devicetree/bindings/misc/fsl,qoriq-mc.txt  | 45
> ++
>  1 file changed, 45 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> index 848975f..6611a7c 100644
> --- a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> +++ b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> @@ -55,6 +55,40 @@ Required properties:
>  Value type: 
>  Definition: Must be 1.
> 
> +Sub-nodes:
> +
> +The fsl-mc node may optionally have dpmac sub-nodes that
> describe
> +the relationship between the Ethernet MACs which belong to the
> MC
> +and the Ethernet PHYs on the system board.
> +
> +The dpmac nodes must be under a node named "dpmacs" which
> contains
> +the following properties:
> +
> +- #address-cells
> +  Value type: 
> +  Definition: Must be present if dpmac sub-nodes are defined
> and must
> +  have a value of 1.
> +
> +- #size-cells
> +  Value type: 
> +  Definition: Must be present if dpmac sub-nodes are defined
> and must
> +  have a value of 0.
> +
> +These nodes must have the following properties:
> +
> +- compatible
> +  Value type: 
> +  Definition: Must be "fsl,qoriq-mc-dpmac".
> +
> +- reg
> +  Value type: 
> +  Definition: Specifies the id of the dpmac.
> +
> +- phy-handle
> +  Value type: 
> +  Definition: Specifies the phandle to the PHY device node
> associated
> +  with the this dpmac.
> +
>  Example:
> 
>  fsl_mc: fsl-mc@80c00 {
> @@ -71,4 +105,15 @@ Example:
>   */
>  ranges = <0x0 0x0 0x0 0x8 0x0c00 0x400
>0x1 0x0 0x0 0x8 0x1800 0x800>;
> +
> +dpmacs {
> +#address-cells = <1>;
> +#size-cells = <0>;
> +
> +dpmac@1 {
> +compatible = "fsl,qoriq-mc-dpmac";
> +reg = <1>;
> +phy-handle = <_phy0>;
> +}
> +}
>  };
> --
> 2.3.3

Acked-by: J. German Rivera 

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

RE: [PATCH 2/3] Docs: dt: fsl-mc update binding to include definition of ranges

[Jose Rivera]

> -Original Message-
> From: Stuart Yoder [mailto:stuart.yo...@freescale.com]
> Sent: Friday, September 25, 2015 4:08 PM
> To: robh...@kernel.org; mark.rutl...@arm.com
> Cc: devicet...@vger.kernel.org; linux-kernel@vger.kernel.org; Rivera
> Jose-B46482; Sharma Bhupesh-B45370; katz Itai-RM05202; Yoder Stuart-
> B08248
> Subject: [PATCH 2/3] Docs: dt: fsl-mc update binding to include
> definition of ranges
> 
> Define a ranges property to specify the mapping between the MC address
> space and the system address space.
> 
> Signed-off-by: Stuart Yoder 
> ---
>  .../devicetree/bindings/misc/fsl,qoriq-mc.txt  | 30
> +-
>  1 file changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> index 6aac955..848975f 100644
> --- a/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> +++ b/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> @@ -35,6 +35,26 @@ Required properties:
>  Definition: Must be present and point to the MSI controller node
>  handling message interrupts for the MC.
> 
> +- ranges
> +Value type: 
> +Definition: A standard property.  Defines the mapping between
> the child
> +MC address space and the parent system address
> space.
> +
> +The MC address space is defined by 3 components:
> + 
> +
> +Valid values for region type are
> +   0x0 - MC portals
> +   0x1 - QBMAN portals
> +
> +- #address-cells
> +Value type: 
> +Definition: Must be 3.  (see definition in 'ranges' property)
> +
> +- #size-cells
> +Value type: 
> +Definition: Must be 1.
> +
>  Example:
> 
>  fsl_mc: fsl-mc@80c00 {
> @@ -42,5 +62,13 @@ Example:
>  reg = <0x0008 0x0c00 0 0x40>,/* MC portal
> base */
><0x 0x0834 0 0x4>; /* MC control
> reg */
>  msi-parent = <>;
> -};
> +#address-cells = <3>;
> +#size-cells = <1>;
> 
> +/*
> + * Region type 0x0 - MC portals
> + * Region type 0x1 - QBMAN portals
> + */
> +ranges = <0x0 0x0 0x0 0x8 0x0c00 0x400
> +  0x1 0x0 0x0 0x8 0x1800 0x800>;
> +};
> --
> 2.3.3

Acked-by: J. German Rivera 

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

RE: [PATCH v2 6/6] staging: fsl-mc: up-rev dprc binary interface to v4.0

[Jose Rivera]

> -Original Message-
> From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
> Sent: Wednesday, September 23, 2015 4:27 AM
> To: Rivera Jose-B46482
> Cc: gre...@linuxfoundation.org; a...@arndb.de;
> de...@driverdev.osuosl.org; linux-kernel@vger.kernel.org; Yoder Stuart-
> B08248; katz Itai-RM05202; Li Yang-Leo-R58472; Wood Scott-B07421;
> ag...@suse.de; Hamciuc Bogdan-BHAMCIU1; Marginean Alexandru-R89243;
> Sharma Bhupesh-B45370; Erez Nir-RM30794; Schmitt Richard-B43082
> Subject: Re: [PATCH v2 6/6] staging: fsl-mc: up-rev dprc binary interface
> to v4.0
> 
> On Tue, Sep 22, 2015 at 06:08:59PM -0500, J. German Rivera wrote:
> >  /**
> > + * dprc_set_obj_label() - Set object label.
> > + * @mc_io: Pointer to MC portal's I/O object
> > + * @cmd_flags: Command flags; one or more of 'MC_CMD_FLAG_'
> > + * @token: Token of DPRC object
> > + * @obj_type:  Object's type
> > + * @obj_id:Object's ID
> > + * @label: The required label. The maximum length is 16 chars.
> > + *
> > + * Return: '0' on Success; Error code otherwise.
> > + */
> > +int dprc_set_obj_label(struct fsl_mc_io*mc_io,
> > +  uint32_t cmd_flags,
> > +  uint16_t token,
> > +  char *obj_type,
> > +  int  obj_id,
> > +  char *label);
> 
> You can fix it in a later patch, but this documentation belongs in the .c
> file instead of the .h file.
> 
Thanks Dan. We will submit a patch for this.

> regards,
> dan carpenter

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

RE: [PATCH v2 6/6] staging: fsl-mc: up-rev dprc binary interface to v4.0

[Jose Rivera]

> -Original Message-
> From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
> Sent: Wednesday, September 23, 2015 4:27 AM
> To: Rivera Jose-B46482
> Cc: gre...@linuxfoundation.org; a...@arndb.de;
> de...@driverdev.osuosl.org; linux-kernel@vger.kernel.org; Yoder Stuart-
> B08248; katz Itai-RM05202; Li Yang-Leo-R58472; Wood Scott-B07421;
> ag...@suse.de; Hamciuc Bogdan-BHAMCIU1; Marginean Alexandru-R89243;
> Sharma Bhupesh-B45370; Erez Nir-RM30794; Schmitt Richard-B43082
> Subject: Re: [PATCH v2 6/6] staging: fsl-mc: up-rev dprc binary interface
> to v4.0
> 
> On Tue, Sep 22, 2015 at 06:08:59PM -0500, J. German Rivera wrote:
> >  /**
> > + * dprc_set_obj_label() - Set object label.
> > + * @mc_io: Pointer to MC portal's I/O object
> > + * @cmd_flags: Command flags; one or more of 'MC_CMD_FLAG_'
> > + * @token: Token of DPRC object
> > + * @obj_type:  Object's type
> > + * @obj_id:Object's ID
> > + * @label: The required label. The maximum length is 16 chars.
> > + *
> > + * Return: '0' on Success; Error code otherwise.
> > + */
> > +int dprc_set_obj_label(struct fsl_mc_io*mc_io,
> > +  uint32_t cmd_flags,
> > +  uint16_t token,
> > +  char *obj_type,
> > +  int  obj_id,
> > +  char *label);
> 
> You can fix it in a later patch, but this documentation belongs in the .c
> file instead of the .h file.
> 
Thanks Dan. We will submit a patch for this.

> regards,
> dan carpenter

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

RE: [PATCH] Staging: fsl-mc/bus: mc-bus.c: Fixed coding style issues

[Jose Rivera]

> -Original Message-
> From: Nayeemahmed Badebade [mailto:itachi.op...@gmail.com]
> Sent: Friday, September 04, 2015 12:01 PM
> To: gre...@linuxfoundation.org
> Cc: Rivera Jose-B46482; Yoder Stuart-B08248; ag...@suse.de;
> hguju...@visteon.com; linux-kernel@vger.kernel.org
> Subject: [PATCH] Staging: fsl-mc/bus: mc-bus.c: Fixed coding style issues
> 
> Fixed coding style issues where kernel types u16,u64,u32 should be
> preferred over uint16_t,uint64_t,uint32_t
> 
Nayeemahmed,

Thanks for your interests in helping us clean up this coding style issue.
However, doing this for all the files related to the MC bus driver (not just
for one) is something that I intend to do in a future patch as
part of the "Cleanup" item of our TODO list.
Doing this clean up across the board for all files is preferable than 
doing it just for this file in this patch.
Thanks,

German

> Signed-off-by: Nayeemahmed Badebade 
> ---
>  drivers/staging/fsl-mc/bus/mc-bus.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/fsl-mc/bus/mc-bus.c b/drivers/staging/fsl-
> mc/bus/mc-bus.c
> index 766a659..b3a60a0 100644
> --- a/drivers/staging/fsl-mc/bus/mc-bus.c
> +++ b/drivers/staging/fsl-mc/bus/mc-bus.c
> @@ -209,7 +209,7 @@ EXPORT_SYMBOL_GPL(fsl_mc_driver_unregister);
>  static int get_dprc_icid(struct fsl_mc_io *mc_io,
>int container_id, uint16_t *icid)
>  {
> - uint16_t dprc_handle;
> + u16 dprc_handle;
>   struct dprc_attributes attr;
>   int error;
> 
> @@ -234,7 +234,7 @@ common_cleanup:
>   return error;
>  }
> 
> -static int translate_mc_addr(uint64_t mc_addr, phys_addr_t *phys_addr)
> +static int translate_mc_addr(u64 mc_addr, phys_addr_t *phys_addr)
>  {
>   int i;
>   struct fsl_mc *mc = dev_get_drvdata(fsl_mc_bus_type.dev_root-
> >parent);
> @@ -600,7 +600,7 @@ static int fsl_mc_bus_probe(struct platform_device
> *pdev)
>   struct fsl_mc_io *mc_io = NULL;
>   int container_id;
>   phys_addr_t mc_portal_phys_addr;
> - uint32_t mc_portal_size;
> + u32 mc_portal_size;
>   struct mc_version mc_version;
>   struct resource res;
> 
> --
> 1.9.1

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

RE: [PATCH] Staging: fsl-mc/bus: mc-bus.c: Fixed coding style issues

[Jose Rivera]

> -Original Message-
> From: Nayeemahmed Badebade [mailto:itachi.op...@gmail.com]
> Sent: Friday, September 04, 2015 12:01 PM
> To: gre...@linuxfoundation.org
> Cc: Rivera Jose-B46482; Yoder Stuart-B08248; ag...@suse.de;
> hguju...@visteon.com; linux-kernel@vger.kernel.org
> Subject: [PATCH] Staging: fsl-mc/bus: mc-bus.c: Fixed coding style issues
> 
> Fixed coding style issues where kernel types u16,u64,u32 should be
> preferred over uint16_t,uint64_t,uint32_t
> 
Nayeemahmed,

Thanks for your interests in helping us clean up this coding style issue.
However, doing this for all the files related to the MC bus driver (not just
for one) is something that I intend to do in a future patch as
part of the "Cleanup" item of our TODO list.
Doing this clean up across the board for all files is preferable than 
doing it just for this file in this patch.
Thanks,

German

> Signed-off-by: Nayeemahmed Badebade 
> ---
>  drivers/staging/fsl-mc/bus/mc-bus.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/fsl-mc/bus/mc-bus.c b/drivers/staging/fsl-
> mc/bus/mc-bus.c
> index 766a659..b3a60a0 100644
> --- a/drivers/staging/fsl-mc/bus/mc-bus.c
> +++ b/drivers/staging/fsl-mc/bus/mc-bus.c
> @@ -209,7 +209,7 @@ EXPORT_SYMBOL_GPL(fsl_mc_driver_unregister);
>  static int get_dprc_icid(struct fsl_mc_io *mc_io,
>int container_id, uint16_t *icid)
>  {
> - uint16_t dprc_handle;
> + u16 dprc_handle;
>   struct dprc_attributes attr;
>   int error;
> 
> @@ -234,7 +234,7 @@ common_cleanup:
>   return error;
>  }
> 
> -static int translate_mc_addr(uint64_t mc_addr, phys_addr_t *phys_addr)
> +static int translate_mc_addr(u64 mc_addr, phys_addr_t *phys_addr)
>  {
>   int i;
>   struct fsl_mc *mc = dev_get_drvdata(fsl_mc_bus_type.dev_root-
> >parent);
> @@ -600,7 +600,7 @@ static int fsl_mc_bus_probe(struct platform_device
> *pdev)
>   struct fsl_mc_io *mc_io = NULL;
>   int container_id;
>   phys_addr_t mc_portal_phys_addr;
> - uint32_t mc_portal_size;
> + u32 mc_portal_size;
>   struct mc_version mc_version;
>   struct resource res;
> 
> --
> 1.9.1

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

RE: [PATCH] staging: fsl-mc: Upgraded MC flibs used in MC bus driver

[Jose Rivera]

> -Original Message-
> From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
> Sent: Thursday, August 20, 2015 2:35 AM
> To: Rivera Jose-B46482
> Cc: gre...@linuxfoundation.org; a...@arndb.de;
> de...@driverdev.osuosl.org; linux-kernel@vger.kernel.org; Yoder Stuart-
> B08248; katz Itai-RM05202; Wood Scott-B07421; ag...@suse.de; Hamciuc
> Bogdan-BHAMCIU1; Marginean Alexandru-R89243; Sharma Bhupesh-B45370; Erez
> Nir-RM30794; Schmitt Richard-B43082
> Subject: Re: [PATCH] staging: fsl-mc: Upgraded MC flibs used in MC bus
> driver
> 
> On Wed, Aug 19, 2015 at 11:52:40AM -0500, J. German Rivera wrote:
> > Since signatures of flib functions have changed, we had to change all
> > the corresponding calls in the MC bus driver
> >
> 
> What does upgrade mean here?  I feel like this is the kind of patch we
> reject without reading the patch because the description is too vague and
> you can tell from the diff stats that it is going to be too huge to
> review.
We will do some refactoring to make this easier to review.

Thanks,

german
> 
> I looked at the first few lines and we are making a ball of changes.
> Copyright notices, adding function parameters, renaming functions.  This
> needs to be explained a lot better and probably split into multiple
> patches.
> 
> Btw, the trees are closed for the next 3-4 weeks until after 4.3-rc2 is
> released.
> 
> regards,
> dan carpenter

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

RE: [PATCH] staging: fsl-mc: Upgraded MC flibs used in MC bus driver

[Jose Rivera]

 -Original Message-
 From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
 Sent: Thursday, August 20, 2015 2:35 AM
 To: Rivera Jose-B46482
 Cc: gre...@linuxfoundation.org; a...@arndb.de;
 de...@driverdev.osuosl.org; linux-kernel@vger.kernel.org; Yoder Stuart-
 B08248; katz Itai-RM05202; Wood Scott-B07421; ag...@suse.de; Hamciuc
 Bogdan-BHAMCIU1; Marginean Alexandru-R89243; Sharma Bhupesh-B45370; Erez
 Nir-RM30794; Schmitt Richard-B43082
 Subject: Re: [PATCH] staging: fsl-mc: Upgraded MC flibs used in MC bus
 driver
 
 On Wed, Aug 19, 2015 at 11:52:40AM -0500, J. German Rivera wrote:
  Since signatures of flib functions have changed, we had to change all
  the corresponding calls in the MC bus driver
 
 
 What does upgrade mean here?  I feel like this is the kind of patch we
 reject without reading the patch because the description is too vague and
 you can tell from the diff stats that it is going to be too huge to
 review.
We will do some refactoring to make this easier to review.

Thanks,

german
 
 I looked at the first few lines and we are making a ball of changes.
 Copyright notices, adding function parameters, renaming functions.  This
 needs to be explained a lot better and probably split into multiple
 patches.
 
 Btw, the trees are closed for the next 3-4 weeks until after 4.3-rc2 is
 released.
 
 regards,
 dan carpenter

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

RE: [PATCH v4 0/7] staging: fsl-mc: New functionality to the MC bus driver

[Jose Rivera]

Greg, 

Thanks for your feedback. It is our fault that we did not articulate
clearly our intent with this patch series. First, for reference, let
us summarize here the patches in question:

Patch 1: MC bus IRQ support
Patch 2: add device binding path 'driver_override'
Patch 3: Propagate driver_override for a child DPRC's children
Patch 4: Upgraded MC bus driver to match MC fw 7.0.0
Patch 5: Allow the MC bus driver to run without GIC support
Patch 6: Add locking to serialize mc_send_command() calls
Patch 7: Use DPMCP IRQ and completion var to wait for MC

With the exception of patches 2 and 3 (needed for vfio), our intent with
the rest was to make to changes to work towards completing the "Add at
least one device driver for a DPAA2 object" on the TODO list.

Before sending further patches we will submit an update to the TODO list
to provide more detail and visibility into our plan to complete the
"Add at least one device driver..." item.  It's too broad as written. 
In particular, we think interrupt support is required and a pre-requisite.

Thanks,

German

> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Friday, June 12, 2015 7:19 PM
> To: Rivera Jose-B46482
> Cc: a...@arndb.de; de...@driverdev.osuosl.org; linux-
> ker...@vger.kernel.org; Yoder Stuart-B08248; Sharma Bhupesh-B45370;
> ag...@suse.de; Hamciuc Bogdan-BHAMCIU1; Erez Nir-RM30794; katz Itai-
> RM05202; Wood Scott-B07421; Marginean Alexandru-R89243;
> dan.carpen...@oracle.com; Schmitt Richard-B43082
> Subject: Re: [PATCH v4 0/7] staging: fsl-mc: New functionality to the MC
> bus driver
> 
> On Tue, Jun 09, 2015 at 04:59:01PM -0500, J. German Rivera wrote:
> > This patch series includes new functionality for the Freescale fsl-mc
> > bus driver.
> 
> Why are people working on "new functionality" instead of working on
> getting this out of the staging tree?  I really hate adding new functions
> to staging drivers, as this is not the correct place for drivers to be in
> the tree.  People should be working to get them out of this location, and
> then you can add new functions to them.
> 
> So I really don't want to take this series, sorry, someone better start
> working on getting this out of drivers/staging/ or I'm going to have to
> drop the driver entirely from the tree.
> 
> thanks,
> 
> greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH v4 0/7] staging: fsl-mc: New functionality to the MC bus driver

[Jose Rivera]

Greg, 

Thanks for your feedback. It is our fault that we did not articulate
clearly our intent with this patch series. First, for reference, let
us summarize here the patches in question:

Patch 1: MC bus IRQ support
Patch 2: add device binding path 'driver_override'
Patch 3: Propagate driver_override for a child DPRC's children
Patch 4: Upgraded MC bus driver to match MC fw 7.0.0
Patch 5: Allow the MC bus driver to run without GIC support
Patch 6: Add locking to serialize mc_send_command() calls
Patch 7: Use DPMCP IRQ and completion var to wait for MC

With the exception of patches 2 and 3 (needed for vfio), our intent with
the rest was to make to changes to work towards completing the Add at
least one device driver for a DPAA2 object on the TODO list.

Before sending further patches we will submit an update to the TODO list
to provide more detail and visibility into our plan to complete the
Add at least one device driver... item.  It's too broad as written. 
In particular, we think interrupt support is required and a pre-requisite.

Thanks,

German

 -Original Message-
 From: Greg KH [mailto:gre...@linuxfoundation.org]
 Sent: Friday, June 12, 2015 7:19 PM
 To: Rivera Jose-B46482
 Cc: a...@arndb.de; de...@driverdev.osuosl.org; linux-
 ker...@vger.kernel.org; Yoder Stuart-B08248; Sharma Bhupesh-B45370;
 ag...@suse.de; Hamciuc Bogdan-BHAMCIU1; Erez Nir-RM30794; katz Itai-
 RM05202; Wood Scott-B07421; Marginean Alexandru-R89243;
 dan.carpen...@oracle.com; Schmitt Richard-B43082
 Subject: Re: [PATCH v4 0/7] staging: fsl-mc: New functionality to the MC
 bus driver
 
 On Tue, Jun 09, 2015 at 04:59:01PM -0500, J. German Rivera wrote:
  This patch series includes new functionality for the Freescale fsl-mc
  bus driver.
 
 Why are people working on new functionality instead of working on
 getting this out of the staging tree?  I really hate adding new functions
 to staging drivers, as this is not the correct place for drivers to be in
 the tree.  People should be working to get them out of this location, and
 then you can add new functions to them.
 
 So I really don't want to take this series, sorry, someone better start
 working on getting this out of drivers/staging/ or I'm going to have to
 drop the driver entirely from the tree.
 
 thanks,
 
 greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH v4 0/7] staging: fsl-mc: New functionality to the MC bus driver

[Jose Rivera]

> -Original Message-
> From: Fabio Estevam [mailto:feste...@gmail.com]
> Sent: Tuesday, June 09, 2015 5:12 PM
> To: Rivera Jose-B46482
> Cc: Greg Kroah-Hartman; Arnd Bergmann; de...@driverdev.osuosl.org; linux-
> kernel; Yoder Stuart-B08248; Sharma Bhupesh-B45370; ag...@suse.de;
> Hamciuc Bogdan-BHAMCIU1; Erez Nir-RM30794; katz Itai-RM05202; Wood Scott-
> B07421; Marginean Alexandru-R89243; Dan Carpenter; Schmitt Richard-B43082
> Subject: Re: [PATCH v4 0/7] staging: fsl-mc: New functionality to the MC
> bus driver
> 
> Hi German,
> 
> On Tue, Jun 9, 2015 at 6:59 PM, J. German Rivera
>  wrote:
> > This patch series includes new functionality for the Freescale fsl-mc
> > bus driver.
> >
> > Patch 1: MC bus IRQ support
> > Patch 2: add device binding path 'driver_override'
> > Patch 3: Propagate driver_override for a child DPRC's children Patch
> > 4: Upgraded MC bus driver to match MC fw 7.0.0 Patch 5: Allow the MC
> > bus driver to run without GIC support Patch 6: Add locking to
> > serialize mc_send_command() calls Patch 7: Use DPMCP IRQ and
> > completion var to wait for MC
> >
> > CHANGE HISTORY
> >
> > Changes in v4:
> > - Addressed comments from Dan Carpenter and Greg Kroah-Hartman.
> >
> > Changes in v3:
> > - Addressed comments from Dan Carpenter.
> >
> > Changes in v2:
> > - Addressed comments from Dan Carpenter and Scott Wood.
> >   Details in each patch.
> 
> Not related to this series, but if you have a chance, please fix this
> section mismatch warning from the fsl-mc driver:
> 
Sure. Thanks for your feedback.

> allmodconfig WARNING:
> drivers/staging/fsl-mc/bus/mc-bus-driver.o(.init.text+0x18c): Section
> mismatch in reference from the function init_module() to the function
> .exit.text:dprc_driver_exit()
N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i

RE: [PATCH v4 0/7] staging: fsl-mc: New functionality to the MC bus driver

[Jose Rivera]

 -Original Message-
 From: Fabio Estevam [mailto:feste...@gmail.com]
 Sent: Tuesday, June 09, 2015 5:12 PM
 To: Rivera Jose-B46482
 Cc: Greg Kroah-Hartman; Arnd Bergmann; de...@driverdev.osuosl.org; linux-
 kernel; Yoder Stuart-B08248; Sharma Bhupesh-B45370; ag...@suse.de;
 Hamciuc Bogdan-BHAMCIU1; Erez Nir-RM30794; katz Itai-RM05202; Wood Scott-
 B07421; Marginean Alexandru-R89243; Dan Carpenter; Schmitt Richard-B43082
 Subject: Re: [PATCH v4 0/7] staging: fsl-mc: New functionality to the MC
 bus driver
 
 Hi German,
 
 On Tue, Jun 9, 2015 at 6:59 PM, J. German Rivera
 german.riv...@freescale.com wrote:
  This patch series includes new functionality for the Freescale fsl-mc
  bus driver.
 
  Patch 1: MC bus IRQ support
  Patch 2: add device binding path 'driver_override'
  Patch 3: Propagate driver_override for a child DPRC's children Patch
  4: Upgraded MC bus driver to match MC fw 7.0.0 Patch 5: Allow the MC
  bus driver to run without GIC support Patch 6: Add locking to
  serialize mc_send_command() calls Patch 7: Use DPMCP IRQ and
  completion var to wait for MC
 
  CHANGE HISTORY
 
  Changes in v4:
  - Addressed comments from Dan Carpenter and Greg Kroah-Hartman.
 
  Changes in v3:
  - Addressed comments from Dan Carpenter.
 
  Changes in v2:
  - Addressed comments from Dan Carpenter and Scott Wood.
Details in each patch.
 
 Not related to this series, but if you have a chance, please fix this
 section mismatch warning from the fsl-mc driver:
 
Sure. Thanks for your feedback.

 allmodconfig WARNING:
 drivers/staging/fsl-mc/bus/mc-bus-driver.o(.init.text+0x18c): Section
 mismatch in reference from the function init_module() to the function
 .exit.text:dprc_driver_exit()
N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z��w���?��)ߢf��^jǫy�m��@A�a���
0��h���i

RE: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support

[Jose Rivera]

> -Original Message-
> From: Wood Scott-B07421
> Sent: Tuesday, May 05, 2015 2:42 PM
> To: Rivera Jose-B46482
> Cc: Dan Carpenter; de...@driverdev.osuosl.org; ag...@suse.de;
> a...@arndb.de; Sharma Bhupesh-B45370; gre...@linuxfoundation.org; linux-
> ker...@vger.kernel.org; Yoder Stuart-B08248; Erez Nir-RM30794; katz Itai-
> RM05202; Hamciuc Bogdan-BHAMCIU1; Marginean Alexandru-R89243; Schmitt
> Richard-B43082
> Subject: Re: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support
> 
> On Tue, 2015-05-05 at 11:08 -0500, Rivera Jose-B46482 wrote:
> > > > > to read what "goto error;" does.  The error handling here calls
> > > > > devm_kfree() which is not needed...  devm_ functions
> > > > > automatically clean up after themselves.  This seems a pattern
> > > > > throughout.  Do a search for
> > > > > devm_free() and see which ones are really needed or not.
> > > > >
> > > > I know that memory allocated with devm_kzalloc() is freed at the
> > > > end of the lifetime of the device it is attached to. However, in
> > > > error paths, why wait until the device is destroyed? Why not free
> > > > the memory earlier so that it can be used for other purposes?
> > >
> > Why then do the devm_kfree() function exist?
> >
> > I will not remove the devm_free() calls unless the upstream maintainer
> > requires me to do so.
> 
> The whole point of devm is to simplify (often poorly tested) error paths.
> You're trying to optimize the error path instead of simplify it, which
> doesn't make sense.
> 
> devm_kfree() is for the uncommon case when you want to free the memory in
> a situation where the device isn't being unbound any time soon, but you
> still want the memory to be handled by devm for some reason.
> 
> Most users of devm_kzalloc() do not use devm_kfree():
> scott@snotra:~/fsl/git/linux/upstream$ git grep devm_kzalloc|wc -l
> 2750
> scott@snotra:~/fsl/git/linux/upstream$ git grep devm_kfree|wc -l
> 118
> 
Ok, I'll remove the devm_kfree() calls.

Thanks,

German

> -Scott
> 

RE: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support

[Jose Rivera]

> -Original Message-
> From: Wood Scott-B07421
> Sent: Tuesday, May 05, 2015 2:57 PM
> To: Dan Carpenter
> Cc: Rivera Jose-B46482; de...@driverdev.osuosl.org; Yoder Stuart-B08248;
> Hamciuc Bogdan-BHAMCIU1; a...@arndb.de; Sharma Bhupesh-B45370;
> gre...@linuxfoundation.org; linux-kernel@vger.kernel.org; ag...@suse.de;
> Erez Nir-RM30794; katz Itai-RM05202; Marginean Alexandru-R89243; Schmitt
> Richard-B43082
> Subject: Re: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support
> 
> On Tue, 2015-05-05 at 19:40 +0300, Dan Carpenter wrote:
> > On Tue, May 05, 2015 at 04:08:49PM +, Jose Rivera wrote:
> > > > Subject: Re: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support
> > > >
> > > > On Mon, May 04, 2015 at 10:09:08PM +, Jose Rivera wrote:
> > > > > > > + WARN_ON((int16_t)irq_count < 0);
> > > > > >
> > > > > > This code is doing "WARN_ON(test_bit(15, (unsigned long
> > > > *)_count));".
> > > > > > That seems like nonsense.  Anyway, just delete the WARN_ON().
> > > > > >
> > > > > I disagree. This WARN_ON is checking that irq_count is in the
> > > > > expected range (it fits in int16_t as a positive number). The
> > > > > dprc_scan_objects() function expects irq_count to be of type
> > > > > "unsigned int" (which is 32-bit unsigned)
> > > > >
> > > >
> > > > You're not allowed to disagree because it's a testable thing and
> > > > not an opinion about style or something.  :P  What you want is:
> > > >
> > > > WARN_ON(irq_count > SHRT_MAX);
> > > >
> > > I see your point now. The check "(int16_t)irq_count < 0)" will not
> > > be able to catch 0x1 > 0x7fff, but "irq_count > SHRT_MAX) will.
> > > So I'll make the suggested change, but I would prefer to use S16_MAX
> > > rather than SHRT_MAX.
> > >
> >
> > Huh?  I didn't even know about the S16_MAX definition.  There are
> > literally no users of it in the kernel.  It's not very fair because
> > there are few users of SHRT_MAX.  But there are literally no users of
> > S32_MAX in the kernel and 358 users of INT_MAX.
> >
> > Don't insist that you must be special and different from everyone else.
> 
> There are some users of U16_MAX, U32_MAX, and U64_MAX.  Why use a limit
> for a different type than is being used?  Why have s16/s32 at all if
> you're going to conflate it with short/int elsewhere?
> 
> That said, I don't see where this code is actually using s16 (or
> int16_t) for irq_count except in these weird error checks.  German, why
> do you need to check against 0x7fff (whatever you call it) at all?
> Won't comparing to a promoted-to-unsigned-int .max_count (as you do
> immediately after the WARN_ON) suffice?
> 
mc_bus->resource_pools[FSL_MC_POOL_IRQ].max_count is of type int16_t
(and is so, because its value comes from an MC API that returns
an int16_t). The reason for checking irq_count against 0x7 is to 
catch the case in which irq_count is out of range (irq_count originates
from values coming from the MC device, so we should do some validation
before using it)

Thanks,

German


> -Scott
> 

N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i

RE: [PATCH 6/7] staging: fsl-mc: Add locking to serialize mc_send_command() calls

[Jose Rivera]

> -Original Message-
> From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
> Sent: Thursday, April 30, 2015 7:59 AM
> To: Rivera Jose-B46482
> Cc: gre...@linuxfoundation.org; a...@arndb.de;
> de...@driverdev.osuosl.org; linux-kernel@vger.kernel.org; Yoder Stuart-
> B08248; Sharma Bhupesh-B45370; ag...@suse.de; Hamciuc Bogdan-BHAMCIU1;
> Erez Nir-RM30794; katz Itai-RM05202; Wood Scott-B07421; Marginean
> Alexandru-R89243; Schmitt Richard-B43082
> Subject: Re: [PATCH 6/7] staging: fsl-mc: Add locking to serialize
> mc_send_command() calls
> 
> On Tue, Apr 28, 2015 at 12:39:09PM -0500, J. German Rivera wrote:
> > @@ -230,15 +235,26 @@ static inline enum mc_cmd_status
> mc_read_response(struct mc_command __iomem *
> >   * @cmd: command to be sent
> >   *
> >   * Returns '0' on Success; Error code otherwise.
> > - *
> > - * NOTE: This function cannot be invoked from from atomic contexts.
> >   */
> >  int mc_send_command(struct fsl_mc_io *mc_io, struct mc_command *cmd)
> > {
> > +   int error;
> > enum mc_cmd_status status;
> > unsigned long jiffies_until_timeout =
> > jiffies + MC_CMD_COMPLETION_TIMEOUT_JIFFIES;
> 
> We busy loop while holding a spinlock for half a second.  That seems bad.
> 
What would be a reasonable max time for holding a spinlock here?

> >
> > +   if (preemptible()) {
> 
> This is wrong.  If the user asked for spinlocks they should always get
> spinlocks.  It shouldn't matter that they are not currently holding a
> different lock.
> 
> I'm skeptical of this locking anyway.
> 
> Also what about if they have PREEMPT disabled?  There aren't any users
> for this stuff anyway so it's impossible to review how people are
> FSL_MC_IO_ATOMIC_CONTEXT_PORTAL.
> 
> Let's wait until there is a user before looking at this.
> 
> > -   return mc_status_to_error(status);
> > +   error = mc_status_to_error(status);
> > +   goto common_exit;
> > }
> >
> > -   return 0;
> > +   error = 0;
> > +
> > +common_exit:
> 
> Just name this unlock:.
> 
> > +   if (preemptible())
> > +   mutex_unlock(_io->mutex);
> > +   else
> > +   spin_unlock(_io->spinlock);
> > +
> > +   return error;
> >  }
> 
> regards,
> dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support

[Jose Rivera]

> -Original Message-
> From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
> Sent: Tuesday, May 05, 2015 3:49 AM
> To: Rivera Jose-B46482
> Cc: de...@driverdev.osuosl.org; ag...@suse.de; a...@arndb.de; Sharma
> Bhupesh-B45370; gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> Yoder Stuart-B08248; Wood Scott-B07421; Erez Nir-RM30794; katz Itai-
> RM05202; Hamciuc Bogdan-BHAMCIU1; Marginean Alexandru-R89243; Schmitt
> Richard-B43082
> Subject: Re: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support
> 
> On Mon, May 04, 2015 at 10:09:08PM +, Jose Rivera wrote:
> > > > +   WARN_ON((int16_t)irq_count < 0);
> > >
> > > This code is doing "WARN_ON(test_bit(15, (unsigned long
> *)_count));".
> > > That seems like nonsense.  Anyway, just delete the WARN_ON().
> > >
> > I disagree. This WARN_ON is checking that irq_count is in the expected
> > range (it fits in int16_t as a positive number). The
> > dprc_scan_objects() function expects irq_count to be of type "unsigned
> > int" (which is 32-bit unsigned)
> >
> 
> You're not allowed to disagree because it's a testable thing and not an
> opinion about style or something.  :P  What you want is:
> 
>   WARN_ON(irq_count > SHRT_MAX);
> 
I see your point now. The check "(int16_t)irq_count < 0)" will not be able
to catch 0x1 > 0x7fff, but "irq_count > SHRT_MAX) will. So I'll
make the suggested change, but I would prefer to use S16_MAX rather than 
SHRT_MAX.

> > > > +
> > > > +   if ((int16_t)irq_count >
> > > > +   mc_bus-
> >resource_pools[FSL_MC_POOL_IRQ].max_count) {
> > >
> > > Why are we casting this?  Also can you align it like:
> > >
> > This casting is done for safety, to prevent the comparison to be done
> > in "unsigned int" due to integer promotion rules.
> 
> We are truncating away the top bytes but then we use them later.
> Fortunately we use them only to print out a warning, but if we used them
> for anything else it would be a serious bug.
> 
> Are you expecting .max_count to be negative?
> 
No.

> If not then both sides are positive and type promotion is fine.  We can
> delete the first (buggy) warning, like I said and just leave the second
> warning.  It will now complain if any of bits 16 to 31 are set where
> before it wouldn't.
> 
Agreed. I'll remove the (int16_t) type cast from the "if". So, I'll change
this code snippet to be like this:

WARN_ON(irq_count > S16_MAX);

if (irq_count >
mc_bus->resource_pools[FSL_MC_POOL_IRQ].max_count) 
dev_warn(...);


Although the WARN_ON seems redundant with the "if", it catches a different
problem. The WARN_ON() catches irq_count to be out of range, the "if"
tells when we run out of IRQ resources fro a valid irq_count.

> > > to read what "goto error;" does.  The error handling here calls
> > > devm_kfree() which is not needed...  devm_ functions automatically
> > > clean up after themselves.  This seems a pattern throughout.  Do a
> > > search for
> > > devm_free() and see which ones are really needed or not.
> > >
> > I know that memory allocated with devm_kzalloc() is freed at the end
> > of the lifetime of the device it is attached to. However, in error
> > paths, why wait until the device is destroyed? Why not free the memory
> > earlier so that it can be used for other purposes?
> 
Why then do the devm_kfree() function exist?

I will not remove the devm_free() calls unless the upstream maintainer
requires me to do so.

> My understanding is that devm_ functions are supposed to be used in the
> probe() functions to simplify the error handling.  So hopefully the
> device lifetime ends as soon as this function returns a failure.
> 
> devm_ function are not a use them everywhere because now the kernel has
> garbage collection type thing.
> 
> regards,
> dan carpenter

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

RE: [PATCH 6/7] staging: fsl-mc: Add locking to serialize mc_send_command() calls

[Jose Rivera]

 -Original Message-
 From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
 Sent: Thursday, April 30, 2015 7:59 AM
 To: Rivera Jose-B46482
 Cc: gre...@linuxfoundation.org; a...@arndb.de;
 de...@driverdev.osuosl.org; linux-kernel@vger.kernel.org; Yoder Stuart-
 B08248; Sharma Bhupesh-B45370; ag...@suse.de; Hamciuc Bogdan-BHAMCIU1;
 Erez Nir-RM30794; katz Itai-RM05202; Wood Scott-B07421; Marginean
 Alexandru-R89243; Schmitt Richard-B43082
 Subject: Re: [PATCH 6/7] staging: fsl-mc: Add locking to serialize
 mc_send_command() calls
 
 On Tue, Apr 28, 2015 at 12:39:09PM -0500, J. German Rivera wrote:
  @@ -230,15 +235,26 @@ static inline enum mc_cmd_status
 mc_read_response(struct mc_command __iomem *
* @cmd: command to be sent
*
* Returns '0' on Success; Error code otherwise.
  - *
  - * NOTE: This function cannot be invoked from from atomic contexts.
*/
   int mc_send_command(struct fsl_mc_io *mc_io, struct mc_command *cmd)
  {
  +   int error;
  enum mc_cmd_status status;
  unsigned long jiffies_until_timeout =
  jiffies + MC_CMD_COMPLETION_TIMEOUT_JIFFIES;
 
 We busy loop while holding a spinlock for half a second.  That seems bad.
 
What would be a reasonable max time for holding a spinlock here?

 
  +   if (preemptible()) {
 
 This is wrong.  If the user asked for spinlocks they should always get
 spinlocks.  It shouldn't matter that they are not currently holding a
 different lock.
 
 I'm skeptical of this locking anyway.
 
 Also what about if they have PREEMPT disabled?  There aren't any users
 for this stuff anyway so it's impossible to review how people are
 FSL_MC_IO_ATOMIC_CONTEXT_PORTAL.
 
 Let's wait until there is a user before looking at this.
 
  -   return mc_status_to_error(status);
  +   error = mc_status_to_error(status);
  +   goto common_exit;
  }
 
  -   return 0;
  +   error = 0;
  +
  +common_exit:
 
 Just name this unlock:.
 
  +   if (preemptible())
  +   mutex_unlock(mc_io-mutex);
  +   else
  +   spin_unlock(mc_io-spinlock);
  +
  +   return error;
   }
 
 regards,
 dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support

[Jose Rivera]

 -Original Message-
 From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
 Sent: Tuesday, May 05, 2015 3:49 AM
 To: Rivera Jose-B46482
 Cc: de...@driverdev.osuosl.org; ag...@suse.de; a...@arndb.de; Sharma
 Bhupesh-B45370; gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
 Yoder Stuart-B08248; Wood Scott-B07421; Erez Nir-RM30794; katz Itai-
 RM05202; Hamciuc Bogdan-BHAMCIU1; Marginean Alexandru-R89243; Schmitt
 Richard-B43082
 Subject: Re: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support
 
 On Mon, May 04, 2015 at 10:09:08PM +, Jose Rivera wrote:
+   WARN_ON((int16_t)irq_count  0);
  
   This code is doing WARN_ON(test_bit(15, (unsigned long
 *)irq_count));.
   That seems like nonsense.  Anyway, just delete the WARN_ON().
  
  I disagree. This WARN_ON is checking that irq_count is in the expected
  range (it fits in int16_t as a positive number). The
  dprc_scan_objects() function expects irq_count to be of type unsigned
  int (which is 32-bit unsigned)
 
 
 You're not allowed to disagree because it's a testable thing and not an
 opinion about style or something.  :P  What you want is:
 
   WARN_ON(irq_count  SHRT_MAX);
 
I see your point now. The check (int16_t)irq_count  0) will not be able
to catch 0x1  0x7fff, but irq_count  SHRT_MAX) will. So I'll
make the suggested change, but I would prefer to use S16_MAX rather than 
SHRT_MAX.

+
+   if ((int16_t)irq_count 
+   mc_bus-
 resource_pools[FSL_MC_POOL_IRQ].max_count) {
  
   Why are we casting this?  Also can you align it like:
  
  This casting is done for safety, to prevent the comparison to be done
  in unsigned int due to integer promotion rules.
 
 We are truncating away the top bytes but then we use them later.
 Fortunately we use them only to print out a warning, but if we used them
 for anything else it would be a serious bug.
 
 Are you expecting .max_count to be negative?
 
No.

 If not then both sides are positive and type promotion is fine.  We can
 delete the first (buggy) warning, like I said and just leave the second
 warning.  It will now complain if any of bits 16 to 31 are set where
 before it wouldn't.
 
Agreed. I'll remove the (int16_t) type cast from the if. So, I'll change
this code snippet to be like this:

WARN_ON(irq_count  S16_MAX);

if (irq_count 
mc_bus-resource_pools[FSL_MC_POOL_IRQ].max_count) 
dev_warn(...);


Although the WARN_ON seems redundant with the if, it catches a different
problem. The WARN_ON() catches irq_count to be out of range, the if
tells when we run out of IRQ resources fro a valid irq_count.

   to read what goto error; does.  The error handling here calls
   devm_kfree() which is not needed...  devm_ functions automatically
   clean up after themselves.  This seems a pattern throughout.  Do a
   search for
   devm_free() and see which ones are really needed or not.
  
  I know that memory allocated with devm_kzalloc() is freed at the end
  of the lifetime of the device it is attached to. However, in error
  paths, why wait until the device is destroyed? Why not free the memory
  earlier so that it can be used for other purposes?
 
Why then do the devm_kfree() function exist?

I will not remove the devm_free() calls unless the upstream maintainer
requires me to do so.

 My understanding is that devm_ functions are supposed to be used in the
 probe() functions to simplify the error handling.  So hopefully the
 device lifetime ends as soon as this function returns a failure.
 
 devm_ function are not a use them everywhere because now the kernel has
 garbage collection type thing.
 
 regards,
 dan carpenter

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

RE: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support

[Jose Rivera]

 -Original Message-
 From: Wood Scott-B07421
 Sent: Tuesday, May 05, 2015 2:57 PM
 To: Dan Carpenter
 Cc: Rivera Jose-B46482; de...@driverdev.osuosl.org; Yoder Stuart-B08248;
 Hamciuc Bogdan-BHAMCIU1; a...@arndb.de; Sharma Bhupesh-B45370;
 gre...@linuxfoundation.org; linux-kernel@vger.kernel.org; ag...@suse.de;
 Erez Nir-RM30794; katz Itai-RM05202; Marginean Alexandru-R89243; Schmitt
 Richard-B43082
 Subject: Re: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support
 
 On Tue, 2015-05-05 at 19:40 +0300, Dan Carpenter wrote:
  On Tue, May 05, 2015 at 04:08:49PM +, Jose Rivera wrote:
Subject: Re: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support
   
On Mon, May 04, 2015 at 10:09:08PM +, Jose Rivera wrote:
   + WARN_ON((int16_t)irq_count  0);
 
  This code is doing WARN_ON(test_bit(15, (unsigned long
*)irq_count));.
  That seems like nonsense.  Anyway, just delete the WARN_ON().
 
 I disagree. This WARN_ON is checking that irq_count is in the
 expected range (it fits in int16_t as a positive number). The
 dprc_scan_objects() function expects irq_count to be of type
 unsigned int (which is 32-bit unsigned)

   
You're not allowed to disagree because it's a testable thing and
not an opinion about style or something.  :P  What you want is:
   
WARN_ON(irq_count  SHRT_MAX);
   
   I see your point now. The check (int16_t)irq_count  0) will not
   be able to catch 0x1  0x7fff, but irq_count  SHRT_MAX) will.
   So I'll make the suggested change, but I would prefer to use S16_MAX
   rather than SHRT_MAX.
  
 
  Huh?  I didn't even know about the S16_MAX definition.  There are
  literally no users of it in the kernel.  It's not very fair because
  there are few users of SHRT_MAX.  But there are literally no users of
  S32_MAX in the kernel and 358 users of INT_MAX.
 
  Don't insist that you must be special and different from everyone else.
 
 There are some users of U16_MAX, U32_MAX, and U64_MAX.  Why use a limit
 for a different type than is being used?  Why have s16/s32 at all if
 you're going to conflate it with short/int elsewhere?
 
 That said, I don't see where this code is actually using s16 (or
 int16_t) for irq_count except in these weird error checks.  German, why
 do you need to check against 0x7fff (whatever you call it) at all?
 Won't comparing to a promoted-to-unsigned-int .max_count (as you do
 immediately after the WARN_ON) suffice?
 
mc_bus-resource_pools[FSL_MC_POOL_IRQ].max_count is of type int16_t
(and is so, because its value comes from an MC API that returns
an int16_t). The reason for checking irq_count against 0x7 is to 
catch the case in which irq_count is out of range (irq_count originates
from values coming from the MC device, so we should do some validation
before using it)

Thanks,

German


 -Scott
 

N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z��w���?��)ߢf��^jǫy�m��@A�a���
0��h���i

RE: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support

[Jose Rivera]

 -Original Message-
 From: Wood Scott-B07421
 Sent: Tuesday, May 05, 2015 2:42 PM
 To: Rivera Jose-B46482
 Cc: Dan Carpenter; de...@driverdev.osuosl.org; ag...@suse.de;
 a...@arndb.de; Sharma Bhupesh-B45370; gre...@linuxfoundation.org; linux-
 ker...@vger.kernel.org; Yoder Stuart-B08248; Erez Nir-RM30794; katz Itai-
 RM05202; Hamciuc Bogdan-BHAMCIU1; Marginean Alexandru-R89243; Schmitt
 Richard-B43082
 Subject: Re: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support
 
 On Tue, 2015-05-05 at 11:08 -0500, Rivera Jose-B46482 wrote:
 to read what goto error; does.  The error handling here calls
 devm_kfree() which is not needed...  devm_ functions
 automatically clean up after themselves.  This seems a pattern
 throughout.  Do a search for
 devm_free() and see which ones are really needed or not.

I know that memory allocated with devm_kzalloc() is freed at the
end of the lifetime of the device it is attached to. However, in
error paths, why wait until the device is destroyed? Why not free
the memory earlier so that it can be used for other purposes?
  
  Why then do the devm_kfree() function exist?
 
  I will not remove the devm_free() calls unless the upstream maintainer
  requires me to do so.
 
 The whole point of devm is to simplify (often poorly tested) error paths.
 You're trying to optimize the error path instead of simplify it, which
 doesn't make sense.
 
 devm_kfree() is for the uncommon case when you want to free the memory in
 a situation where the device isn't being unbound any time soon, but you
 still want the memory to be handled by devm for some reason.
 
 Most users of devm_kzalloc() do not use devm_kfree():
 scott@snotra:~/fsl/git/linux/upstream$ git grep devm_kzalloc|wc -l
 2750
 scott@snotra:~/fsl/git/linux/upstream$ git grep devm_kfree|wc -l
 118
 
Ok, I'll remove the devm_kfree() calls.

Thanks,

German

 -Scott
 

RE: [PATCH 4/7] staging: fsl-mc: Upgraded MC bus driver to match MC fw 7.0.0

[Jose Rivera]

> -Original Message-
> From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
> Sent: Thursday, April 30, 2015 7:13 AM
> To: Rivera Jose-B46482
> Cc: gre...@linuxfoundation.org; a...@arndb.de;
> de...@driverdev.osuosl.org; linux-kernel@vger.kernel.org; Yoder Stuart-
> B08248; Sharma Bhupesh-B45370; ag...@suse.de; Hamciuc Bogdan-BHAMCIU1;
> Erez Nir-RM30794; katz Itai-RM05202; Wood Scott-B07421; Marginean
> Alexandru-R89243; Schmitt Richard-B43082
> Subject: Re: [PATCH 4/7] staging: fsl-mc: Upgraded MC bus driver to match
> MC fw 7.0.0
> 
> On Tue, Apr 28, 2015 at 12:39:07PM -0500, J. German Rivera wrote:
> > - Migrated MC bus driver to use DPRC flib 0.6.
> 
> What does this mean?  What is a flib?
> 
The DPRC flib is the API to manipulate DPRC objects. 

> After reading the patch, apparently it means that we can remove all the
> ifdefs from patch 1.  :)
> 
No, we cannot as the required GIC-ITS support is not upstream yet.
I added some of the #ifdefs back that were removed by mistake.

> > - Changed IRQ setup infrastructure to be able to program MSIs
> >   for MC objects in an object-independent way.
> 
> Are these two things related?
> 
No.

> Why does this patch add so many new EXPORTS()s?
> 
Removed new EXPORTs added as they are not needed yet.

> regards,
> dan carpenter

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

RE: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support

[Jose Rivera]

Dan,

Thanks for your comments. My replies inline.

Thanks,

German

> -Original Message-
> From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
> Sent: Thursday, April 30, 2015 6:50 AM
> To: Rivera Jose-B46482
> Cc: gre...@linuxfoundation.org; a...@arndb.de;
> de...@driverdev.osuosl.org; linux-kernel@vger.kernel.org; Yoder Stuart-
> B08248; Sharma Bhupesh-B45370; ag...@suse.de; Hamciuc Bogdan-BHAMCIU1;
> Erez Nir-RM30794; katz Itai-RM05202; Wood Scott-B07421; Marginean
> Alexandru-R89243; Schmitt Richard-B43082
> Subject: Re: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support
> 
> On Tue, Apr 28, 2015 at 12:39:04PM -0500, J. German Rivera wrote:
> > Change-Id: I2a986c465989c3811de19cfe9ed0b77168250cb1
> > Reviewed-on: http://git.am.freescale.net:8181/33626
> > Tested-by: Review Code-CDREVIEW 
> 
> These things are totally useless to the rest of us.  Don't add them.
> 
Agreed. I'll remove them in the next respin.

> 
> > diff --git a/drivers/staging/fsl-mc/TODO b/drivers/staging/fsl-mc/TODO
> > index d78288b..1b8868d 100644
> > --- a/drivers/staging/fsl-mc/TODO
> > +++ b/drivers/staging/fsl-mc/TODO
> > @@ -8,6 +8,9 @@
> >  * Add at least one device driver for a DPAA2 object (child device of
> the
> >fsl-mc bus).
> >
> > +* Enable code blocks guarded by #ifdef GIC_ITS_MC_SUPPORT, when
> > +GIC-ITS
> > +  support for the MC MSIs gets merged.
> > +
> 
> When will that be?  I'd really prefer to not add these ifdeffed bits at
> all.
> 
The owner of the GIC-ITS support patches would need to answer the "when"
question. These #ifdefs are needed to be able to make the code compile
without the GIC-ITS support in place. Of course, the code will not be moved
out of staging with these #ifdefs. There is already an item for this
in the drivers/staging/fsl-mc/TODO file.

> > +   if (status & (DPRC_IRQ_EVENT_OBJ_ADDED |
> > + DPRC_IRQ_EVENT_OBJ_REMOVED |
> > + DPRC_IRQ_EVENT_CONTAINER_DESTROYED |
> > + DPRC_IRQ_EVENT_OBJ_DESTROYED |
> > + DPRC_IRQ_EVENT_OBJ_CREATED)) {
> > +   unsigned int irq_count;
> > +
> > +   error = dprc_scan_objects(mc_dev, _count);
> > +   if (error < 0) {
> > +   dev_err(dev, "dprc_scan_objects() failed: %d\n",
> error);
> > +   goto out;
> > +   }
> > +
> > +   WARN_ON((int16_t)irq_count < 0);
> 
> This code is doing "WARN_ON(test_bit(15, (unsigned long *)_count));".
> That seems like nonsense.  Anyway, just delete the WARN_ON().
> 
I disagree. This WARN_ON is checking that irq_count is in the expected range
(it fits in int16_t as a positive number). The dprc_scan_objects() function
expects irq_count to be of type "unsigned int" (which is 32-bit unsigned)

> > +
> > +   if ((int16_t)irq_count >
> > +   mc_bus->resource_pools[FSL_MC_POOL_IRQ].max_count) {
> 
> Why are we casting this?  Also can you align it like:
> 
This casting is done for safety, to prevent the comparison to be done
in "unsigned int" due to integer promotion rules.

>   if (irq_count >
>   mc_bus->resource_pools[FSL_MC_POOL_IRQ].max_count) {
> 
> [tab][tab][space][space][space][space]mc_bus->resource_pools[
> 
> That way you can tell the if condition from the indented block.  Plus
> that is standard kernel indenting style these days.
> 
Agreed. I'll change this in the next respin.

> 
> [ snip ]
> 
> > +   irqs = devm_kzalloc(_dev->dev, irq_count * sizeof(irqs[0]),
> > +   GFP_KERNEL);
> > +   if (!irqs) {
> > +   error = -ENOMEM;
> > +   dev_err(_dev->dev, "No memory to allocate irqs[]\n");
> > +   goto error;
> 
> I kind of hate One Err Style error handling, because the error labels are
> so general...  You can never guess the point of it until you scroll down
>
Agreed. I will replace the generic error cleanup exit point with
error-specific cleanup exit points, and return directly when no cleanup
is needed.

> to read what "goto error;" does.  The error handling here calls
> devm_kfree() which is not needed...  devm_ functions automatically clean
> up after themselves.  This seems a pattern throughout.  Do a search for
> devm_free() and see which ones are really needed or not.
> 
I know that memory allocated with devm_kzalloc() is freed at the end of the
lifetime of the device it is attached to. However, in error paths, why wait 
until the device is destroyed? Why not free the memory earlier so that it
can be used for other purposes? 

> Also the error message isn't needed here.  kzalloc() has its own better
> error messages built-in.  Adding these error messages which will never be
> printed is just a waste of RAM.
> 
Agreed. Error message removed.

> In other words this should look like:
> 
>   irqs = devm_kcalloc(_dev->dev, sizeof(*irqs), irq_count,
>   GFP_KERNEL);
>   if (!irqs)
>   return -ENOMEM;
> 
> regards,
> dan carpenter

--
To 

RE: [PATCH 4/7] staging: fsl-mc: Upgraded MC bus driver to match MC fw 7.0.0

[Jose Rivera]

 -Original Message-
 From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
 Sent: Thursday, April 30, 2015 7:13 AM
 To: Rivera Jose-B46482
 Cc: gre...@linuxfoundation.org; a...@arndb.de;
 de...@driverdev.osuosl.org; linux-kernel@vger.kernel.org; Yoder Stuart-
 B08248; Sharma Bhupesh-B45370; ag...@suse.de; Hamciuc Bogdan-BHAMCIU1;
 Erez Nir-RM30794; katz Itai-RM05202; Wood Scott-B07421; Marginean
 Alexandru-R89243; Schmitt Richard-B43082
 Subject: Re: [PATCH 4/7] staging: fsl-mc: Upgraded MC bus driver to match
 MC fw 7.0.0
 
 On Tue, Apr 28, 2015 at 12:39:07PM -0500, J. German Rivera wrote:
  - Migrated MC bus driver to use DPRC flib 0.6.
 
 What does this mean?  What is a flib?
 
The DPRC flib is the API to manipulate DPRC objects. 

 After reading the patch, apparently it means that we can remove all the
 ifdefs from patch 1.  :)
 
No, we cannot as the required GIC-ITS support is not upstream yet.
I added some of the #ifdefs back that were removed by mistake.

  - Changed IRQ setup infrastructure to be able to program MSIs
for MC objects in an object-independent way.
 
 Are these two things related?
 
No.

 Why does this patch add so many new EXPORTS()s?
 
Removed new EXPORTs added as they are not needed yet.

 regards,
 dan carpenter

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

RE: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support

[Jose Rivera]

Dan,

Thanks for your comments. My replies inline.

Thanks,

German

 -Original Message-
 From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
 Sent: Thursday, April 30, 2015 6:50 AM
 To: Rivera Jose-B46482
 Cc: gre...@linuxfoundation.org; a...@arndb.de;
 de...@driverdev.osuosl.org; linux-kernel@vger.kernel.org; Yoder Stuart-
 B08248; Sharma Bhupesh-B45370; ag...@suse.de; Hamciuc Bogdan-BHAMCIU1;
 Erez Nir-RM30794; katz Itai-RM05202; Wood Scott-B07421; Marginean
 Alexandru-R89243; Schmitt Richard-B43082
 Subject: Re: [PATCH 1/7] staging: fsl-mc: MC bus IRQ support
 
 On Tue, Apr 28, 2015 at 12:39:04PM -0500, J. German Rivera wrote:
  Change-Id: I2a986c465989c3811de19cfe9ed0b77168250cb1
  Reviewed-on: http://git.am.freescale.net:8181/33626
  Tested-by: Review Code-CDREVIEW cdrev...@freescale.com
 
 These things are totally useless to the rest of us.  Don't add them.
 
Agreed. I'll remove them in the next respin.

 
  diff --git a/drivers/staging/fsl-mc/TODO b/drivers/staging/fsl-mc/TODO
  index d78288b..1b8868d 100644
  --- a/drivers/staging/fsl-mc/TODO
  +++ b/drivers/staging/fsl-mc/TODO
  @@ -8,6 +8,9 @@
   * Add at least one device driver for a DPAA2 object (child device of
 the
 fsl-mc bus).
 
  +* Enable code blocks guarded by #ifdef GIC_ITS_MC_SUPPORT, when
  +GIC-ITS
  +  support for the MC MSIs gets merged.
  +
 
 When will that be?  I'd really prefer to not add these ifdeffed bits at
 all.
 
The owner of the GIC-ITS support patches would need to answer the when
question. These #ifdefs are needed to be able to make the code compile
without the GIC-ITS support in place. Of course, the code will not be moved
out of staging with these #ifdefs. There is already an item for this
in the drivers/staging/fsl-mc/TODO file.

  +   if (status  (DPRC_IRQ_EVENT_OBJ_ADDED |
  + DPRC_IRQ_EVENT_OBJ_REMOVED |
  + DPRC_IRQ_EVENT_CONTAINER_DESTROYED |
  + DPRC_IRQ_EVENT_OBJ_DESTROYED |
  + DPRC_IRQ_EVENT_OBJ_CREATED)) {
  +   unsigned int irq_count;
  +
  +   error = dprc_scan_objects(mc_dev, irq_count);
  +   if (error  0) {
  +   dev_err(dev, dprc_scan_objects() failed: %d\n,
 error);
  +   goto out;
  +   }
  +
  +   WARN_ON((int16_t)irq_count  0);
 
 This code is doing WARN_ON(test_bit(15, (unsigned long *)irq_count));.
 That seems like nonsense.  Anyway, just delete the WARN_ON().
 
I disagree. This WARN_ON is checking that irq_count is in the expected range
(it fits in int16_t as a positive number). The dprc_scan_objects() function
expects irq_count to be of type unsigned int (which is 32-bit unsigned)

  +
  +   if ((int16_t)irq_count 
  +   mc_bus-resource_pools[FSL_MC_POOL_IRQ].max_count) {
 
 Why are we casting this?  Also can you align it like:
 
This casting is done for safety, to prevent the comparison to be done
in unsigned int due to integer promotion rules.

   if (irq_count 
   mc_bus-resource_pools[FSL_MC_POOL_IRQ].max_count) {
 
 [tab][tab][space][space][space][space]mc_bus-resource_pools[
 
 That way you can tell the if condition from the indented block.  Plus
 that is standard kernel indenting style these days.
 
Agreed. I'll change this in the next respin.

 
 [ snip ]
 
  +   irqs = devm_kzalloc(mc_dev-dev, irq_count * sizeof(irqs[0]),
  +   GFP_KERNEL);
  +   if (!irqs) {
  +   error = -ENOMEM;
  +   dev_err(mc_dev-dev, No memory to allocate irqs[]\n);
  +   goto error;
 
 I kind of hate One Err Style error handling, because the error labels are
 so general...  You can never guess the point of it until you scroll down

Agreed. I will replace the generic error cleanup exit point with
error-specific cleanup exit points, and return directly when no cleanup
is needed.

 to read what goto error; does.  The error handling here calls
 devm_kfree() which is not needed...  devm_ functions automatically clean
 up after themselves.  This seems a pattern throughout.  Do a search for
 devm_free() and see which ones are really needed or not.
 
I know that memory allocated with devm_kzalloc() is freed at the end of the
lifetime of the device it is attached to. However, in error paths, why wait 
until the device is destroyed? Why not free the memory earlier so that it
can be used for other purposes? 

 Also the error message isn't needed here.  kzalloc() has its own better
 error messages built-in.  Adding these error messages which will never be
 printed is just a waste of RAM.
 
Agreed. Error message removed.

 In other words this should look like:
 
   irqs = devm_kcalloc(mc_dev-dev, sizeof(*irqs), irq_count,
   GFP_KERNEL);
   if (!irqs)
   return -ENOMEM;
 
 regards,
 dan carpenter

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More