Hello Rasmus, I have seen a lot of fsl soc patches from you.
Could you please post what you have modified? Did you test your patches on FSL SoC machines? Thanks, Christian Sent from my iPhone > On 28. Nov 2019, at 16:00, linuxppc-dev-requ...@lists.ozlabs.org wrote: > > Send Linuxppc-dev mailing list submissions to > linuxppc-dev@lists.ozlabs.org > > To subscribe or unsubscribe via the World Wide Web, visit > https://lists.ozlabs.org/listinfo/linuxppc-dev > or, via email, send a message with subject or body 'help' to > linuxppc-dev-requ...@lists.ozlabs.org > > You can reach the person managing the list at > linuxppc-dev-ow...@lists.ozlabs.org > > When replying, please edit your Subject line so it is more specific > than "Re: Contents of Linuxppc-dev digest..." > > > Today's Topics: > > 1. [PATCH v6 36/49] soc: fsl: qe: make cpm_muram_free() return > void (Rasmus Villemoes) > 2. [PATCH v6 37/49] soc: fsl: qe: make cpm_muram_free() ignore a > negative offset (Rasmus Villemoes) > 3. [PATCH v6 38/49] soc: fsl: qe: drop broken lazy call of > cpm_muram_init() (Rasmus Villemoes) > 4. [PATCH v6 39/49] soc: fsl: qe: refactor > cpm_muram_alloc_common to prevent BUG on error path (Rasmus Villemoes) > 5. [PATCH v6 40/49] soc: fsl: qe: avoid IS_ERR_VALUE in > ucc_slow.c (Rasmus Villemoes) > > > ---------------------------------------------------------------------- > > Message: 1 > Date: Thu, 28 Nov 2019 15:55:41 +0100 > From: Rasmus Villemoes <li...@rasmusvillemoes.dk> > To: Qiang Zhao <qiang.z...@nxp.com>, Li Yang <leoyang...@nxp.com>, > Christophe Leroy <christophe.le...@c-s.fr> > Cc: linuxppc-dev@lists.ozlabs.org, > linux-arm-ker...@lists.infradead.org, linux-ker...@vger.kernel.org, > Scott Wood <o...@buserror.net>, Timur Tabi <ti...@kernel.org>, Rasmus > Villemoes <li...@rasmusvillemoes.dk> > Send Linuxppc-dev mailing list submissions to > linuxppc-dev@lists.ozlabs.org > > To subscribe or unsubscribe via the World Wide Web, visit > https://lists.ozlabs.org/listinfo/linuxppc-dev > or, via email, send a message with subject or body 'help' to > linuxppc-dev-requ...@lists.ozlabs.org > > You can reach the person managing the list at > linuxppc-dev-ow...@lists.ozlabs.org > > When replying, please edit your Subject line so it is more specific > than "Re: Contents of Linuxppc-dev digest..." > > > Today's Topics: > > 1. [PATCH v6 36/49] soc: fsl: qe: make cpm_muram_free() return > void (Rasmus Villemoes) > 2. [PATCH v6 37/49] soc: fsl: qe: make cpm_muram_free() ignore a > negative offset (Rasmus Villemoes) > 3. [PATCH v6 38/49] soc: fsl: qe: drop broken lazy call of > cpm_muram_init() (Rasmus Villemoes) > 4. [PATCH v6 39/49] soc: fsl: qe: refactor > cpm_muram_alloc_common to prevent BUG on error path (Rasmus Villemoes) > 5. [PATCH v6 40/49] soc: fsl: qe: avoid IS_ERR_VALUE in > ucc_slow.c (Rasmus Villemoes) > > > ---------------------------------------------------------------------- > > Message: 1 > Date: Thu, 28 Nov 2019 15:55:41 +0100 > From: Rasmus Villemoes <li...@rasmusvillemoes.dk> > To: Qiang Zhao <qiang.z...@nxp.com>, Li Yang <leoyang...@nxp.com>, > Christophe Leroy <christophe.le...@c-s.fr> > Cc: linuxppc-dev@lists.ozlabs.org, > linux-arm-ker...@lists.infradead.org, linux-ker...@vger.kernel.org, > Scott Wood <o...@buserror.net>, Timur Tabi <ti...@kernel.org>, Rasmus > Villemoes <li...@rasmusvillemoes.dk> > Subject: [PATCH v6 36/49] soc: fsl: qe: make cpm_muram_free() return > void > Message-ID: <20191128145554.1297-37-li...@rasmusvillemoes.dk> > > Nobody uses the return value from cpm_muram_free, and functions that > free resources usually return void. One could imagine a use for a "how > much have I allocated" a la ksize(), but knowing how much one had > access to after the fact is useless. > > Reviewed-by: Timur Tabi <ti...@kernel.org> > Signed-off-by: Rasmus Villemoes <li...@rasmusvillemoes.dk> > --- > drivers/soc/fsl/qe/qe_common.c | 3 +-- > include/soc/fsl/qe/qe.h | 5 ++--- > 2 files changed, 3 insertions(+), 5 deletions(-) > > diff --git a/drivers/soc/fsl/qe/qe_common.c b/drivers/soc/fsl/qe/qe_common.c > index 84c90105e588..962835488f66 100644 > --- a/drivers/soc/fsl/qe/qe_common.c > +++ b/drivers/soc/fsl/qe/qe_common.c > @@ -170,7 +170,7 @@ EXPORT_SYMBOL(cpm_muram_alloc); > * cpm_muram_free - free a chunk of multi-user ram > * @offset: The beginning of the chunk as returned by cpm_muram_alloc(). > */ > -int cpm_muram_free(s32 offset) > +void cpm_muram_free(s32 offset) > { > unsigned long flags; > int size; > @@ -188,7 +188,6 @@ int cpm_muram_free(s32 offset) > } > gen_pool_free(muram_pool, offset + GENPOOL_OFFSET, size); > spin_unlock_irqrestore(&cpm_muram_lock, flags); > - return size; > } > EXPORT_SYMBOL(cpm_muram_free); > > diff --git a/include/soc/fsl/qe/qe.h b/include/soc/fsl/qe/qe.h > index f589ae3f1216..e282ac01ec08 100644 > --- a/include/soc/fsl/qe/qe.h > +++ b/include/soc/fsl/qe/qe.h > @@ -99,7 +99,7 @@ int cpm_muram_init(void); > > #if defined(CONFIG_CPM) || defined(CONFIG_QUICC_ENGINE) > s32 cpm_muram_alloc(unsigned long size, unsigned long align); > -int cpm_muram_free(s32 offset); > +void cpm_muram_free(s32 offset); > s32 cpm_muram_alloc_fixed(unsigned long offset, unsigned long size); > void __iomem *cpm_muram_addr(unsigned long offset); > unsigned long cpm_muram_offset(void __iomem *addr); > @@ -111,9 +111,8 @@ static inline s32 cpm_muram_alloc(unsigned long size, > return -ENOSYS; > } > > -static inline int cpm_muram_free(s32 offset) > +static inline void cpm_muram_free(s32 offset) > { > - return -ENOSYS; > } > > static inline s32 cpm_muram_alloc_fixed(unsigned long offset, > -- > 2.23.0 > > > > ------------------------------ > > Message: 2 > Date: Thu, 28 Nov 2019 15:55:42 +0100 > From: Rasmus Villemoes <li...@rasmusvillemoes.dk> > To: Qiang Zhao <qiang.z...@nxp.com>, Li Yang <leoyang...@nxp.com>, > Christophe Leroy <christophe.le...@c-s.fr> > Cc: linuxppc-dev@lists.ozlabs.org, > linux-arm-ker...@lists.infradead.org, linux-ker...@vger.kernel.org, > Scott Wood <o...@buserror.net>, Timur Tabi <ti...@kernel.org>, Rasmus > Villemoes <li...@rasmusvillemoes.dk> > Subject: [PATCH v6 37/49] soc: fsl: qe: make cpm_muram_free() ignore a > negative offset > Message-ID: <20191128145554.1297-38-li...@rasmusvillemoes.dk> > > This allows one to simplify callers since they can store a negative > value as a sentinel to indicate "this was never allocated" (or store > the -ENOMEM from an allocation failure) and then call cpm_muram_free() > unconditionally. > > Reviewed-by: Timur Tabi <ti...@kernel.org> > Signed-off-by: Rasmus Villemoes <li...@rasmusvillemoes.dk> > --- > drivers/soc/fsl/qe/qe_common.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/soc/fsl/qe/qe_common.c b/drivers/soc/fsl/qe/qe_common.c > index 962835488f66..48c77bb92846 100644 > --- a/drivers/soc/fsl/qe/qe_common.c > +++ b/drivers/soc/fsl/qe/qe_common.c > @@ -176,6 +176,9 @@ void cpm_muram_free(s32 offset) > int size; > struct muram_block *tmp; > > + if (offset < 0) > + return; > + > size = 0; > spin_lock_irqsave(&cpm_muram_lock, flags); > list_for_each_entry(tmp, &muram_block_list, head) { > -- > 2.23.0 > > > > ------------------------------ > > Message: 3 > Date: Thu, 28 Nov 2019 15:55:43 +0100 > From: Rasmus Villemoes <li...@rasmusvillemoes.dk> > To: Qiang Zhao <qiang.z...@nxp.com>, Li Yang <leoyang...@nxp.com>, > Christophe Leroy <christophe.le...@c-s.fr> > Cc: linuxppc-dev@lists.ozlabs.org, > linux-arm-ker...@lists.infradead.org, linux-ker...@vger.kernel.org, > Scott Wood <o...@buserror.net>, Timur Tabi <ti...@kernel.org>, Rasmus > Villemoes <li...@rasmusvillemoes.dk> > Subject: [PATCH v6 38/49] soc: fsl: qe: drop broken lazy call of > cpm_muram_init() > Message-ID: <20191128145554.1297-39-li...@rasmusvillemoes.dk> > > cpm_muram_alloc_common() tries to support a kind of lazy > initialization - if the muram_pool has not been created yet, it calls > cpm_muram_init(). Now, cpm_muram_alloc_common() is always called under > > spin_lock_irqsave(&cpm_muram_lock, flags); > > and cpm_muram_init() does gen_pool_create() (which implies a > GFP_KERNEL allocation) and ioremap(), not to mention the fun that > ensues from cpm_muram_init() doing > > spin_lock_init(&cpm_muram_lock); > > In other words, this has never worked, so nobody can have been relying > on it. > > cpm_muram_init() is called from a subsys_initcall (either from > cpm_init() in arch/powerpc/sysdev/cpm_common.c or, via qe_reset(), > from qe_init() in drivers/soc/fsl/qe/qe.c). > > Reviewed-by: Timur Tabi <ti...@kernel.org> > Signed-off-by: Rasmus Villemoes <li...@rasmusvillemoes.dk> > --- > drivers/soc/fsl/qe/qe_common.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/drivers/soc/fsl/qe/qe_common.c b/drivers/soc/fsl/qe/qe_common.c > index 48c77bb92846..dcb267567d76 100644 > --- a/drivers/soc/fsl/qe/qe_common.c > +++ b/drivers/soc/fsl/qe/qe_common.c > @@ -119,9 +119,6 @@ static s32 cpm_muram_alloc_common(unsigned long size, > struct muram_block *entry; > s32 start; > > - if (!muram_pool && cpm_muram_init()) > - goto out2; > - > start = gen_pool_alloc_algo(muram_pool, size, algo, data); > if (!start) > goto out2; > -- > 2.23.0 > > > > ------------------------------ > > Message: 4 > Date: Thu, 28 Nov 2019 15:55:44 +0100 > From: Rasmus Villemoes <li...@rasmusvillemoes.dk> > To: Qiang Zhao <qiang.z...@nxp.com>, Li Yang <leoyang...@nxp.com>, > Christophe Leroy <christophe.le...@c-s.fr> > Cc: linuxppc-dev@lists.ozlabs.org, > linux-arm-ker...@lists.infradead.org, linux-ker...@vger.kernel.org, > Scott Wood <o...@buserror.net>, Timur Tabi <ti...@kernel.org>, Rasmus > Villemoes <li...@rasmusvillemoes.dk> > Subject: [PATCH v6 39/49] soc: fsl: qe: refactor > cpm_muram_alloc_common to prevent BUG on error path > Message-ID: <20191128145554.1297-40-li...@rasmusvillemoes.dk> > > If the kmalloc() fails, we try to undo the gen_pool allocation we've > just done. Unfortunately, start has already been modified to subtract > the GENPOOL_OFFSET bias, so we're freeing something that very likely > doesn't exist in the gen_pool, meaning we hit the > > kernel BUG at lib/genalloc.c:399! > Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM > ... > [<803fd0e8>] (gen_pool_free) from [<80426bc8>] > (cpm_muram_alloc_common+0xb0/0xc8) > [<80426bc8>] (cpm_muram_alloc_common) from [<80426c28>] > (cpm_muram_alloc+0x48/0x80) > [<80426c28>] (cpm_muram_alloc) from [<80428214>] (ucc_slow_init+0x110/0x4f0) > [<80428214>] (ucc_slow_init) from [<8044a718>] > (qe_uart_request_port+0x3c/0x1d8) > > (this was tested by just injecting a random failure by adding > "|| (get_random_int()&7) == 0" to the "if (!entry)" condition). > > Refactor the code so we do the kmalloc() first, meaning that's the > thing that needs undoing in case gen_pool_alloc_algo() then > fails. This allows a later cleanup to move the locking from the > callers into the _common function, keeping the kmalloc() out of the > critical region and then, hopefully (if all the muram_alloc callers > allow) change it to a GFP_KERNEL allocation. > > Reviewed-by: Timur Tabi <ti...@kernel.org> > Signed-off-by: Rasmus Villemoes <li...@rasmusvillemoes.dk> > --- > drivers/soc/fsl/qe/qe_common.c | 16 +++++++--------- > 1 file changed, 7 insertions(+), 9 deletions(-) > > diff --git a/drivers/soc/fsl/qe/qe_common.c b/drivers/soc/fsl/qe/qe_common.c > index dcb267567d76..a81a1a79f1ca 100644 > --- a/drivers/soc/fsl/qe/qe_common.c > +++ b/drivers/soc/fsl/qe/qe_common.c > @@ -119,23 +119,21 @@ static s32 cpm_muram_alloc_common(unsigned long size, > struct muram_block *entry; > s32 start; > > + entry = kmalloc(sizeof(*entry), GFP_ATOMIC); > + if (!entry) > + return -ENOMEM; > start = gen_pool_alloc_algo(muram_pool, size, algo, data); > - if (!start) > - goto out2; > + if (!start) { > + kfree(entry); > + return -ENOMEM; > + } > start = start - GENPOOL_OFFSET; > memset_io(cpm_muram_addr(start), 0, size); > - entry = kmalloc(sizeof(*entry), GFP_ATOMIC); > - if (!entry) > - goto out1; > entry->start = start; > entry->size = size; > list_add(&entry->head, &muram_block_list); > > return start; > -out1: > - gen_pool_free(muram_pool, start, size); > -out2: > - return -ENOMEM; > } > > /* > -- > 2.23.0 > > > > ------------------------------ > > Message: 5 > Date: Thu, 28 Nov 2019 15:55:45 +0100 > From: Rasmus Villemoes <li...@rasmusvillemoes.dk> > To: Qiang Zhao <qiang.z...@nxp.com>, Li Yang <leoyang...@nxp.com>, > Christophe Leroy <christophe.le...@c-s.fr> > Cc: linuxppc-dev@lists.ozlabs.org, > linux-arm-ker...@lists.infradead.org, linux-ker...@vger.kernel.org, > Scott Wood <o...@buserror.net>, Timur Tabi <ti...@kernel.org>, Rasmus > Villemoes <li...@rasmusvillemoes.dk> > Subject: [PATCH v6 40/49] soc: fsl: qe: avoid IS_ERR_VALUE in > ucc_slow.c > Message-ID: <20191128145554.1297-41-li...@rasmusvillemoes.dk> > > When trying to build this for a 64-bit platform, one gets warnings > from using IS_ERR_VALUE on something which is not sizeof(long). > > Instead, change the various *_offset fields to store a signed integer, > and simply check for a negative return from qe_muram_alloc(). Since > qe_muram_free() now accepts and ignores a negative argument, we only > need to make sure these fields are initialized with -1, and we can > just unconditionally call qe_muram_free() in ucc_slow_free(). > > Note that the error case for us_pram_offset failed to set that field > to 0 (which, as noted earlier, is anyway a bogus sentinel value). > > Reviewed-by: Timur Tabi <ti...@kernel.org> > Signed-off-by: Rasmus Villemoes <li...@rasmusvillemoes.dk> > --- > drivers/soc/fsl/qe/ucc_slow.c | 22 +++++++++------------- > include/soc/fsl/qe/ucc_slow.h | 6 +++--- > 2 files changed, 12 insertions(+), 16 deletions(-) > > diff --git a/drivers/soc/fsl/qe/ucc_slow.c b/drivers/soc/fsl/qe/ucc_slow.c > index 9b55fd0f50c6..274d34449846 100644 > --- a/drivers/soc/fsl/qe/ucc_slow.c > +++ b/drivers/soc/fsl/qe/ucc_slow.c > @@ -154,6 +154,9 @@ int ucc_slow_init(struct ucc_slow_info * us_info, struct > ucc_slow_private ** ucc > __func__); > return -ENOMEM; > } > + uccs->rx_base_offset = -1; > + uccs->tx_base_offset = -1; > + uccs->us_pram_offset = -1; > > /* Fill slow UCC structure */ > uccs->us_info = us_info; > @@ -179,7 +182,7 @@ int ucc_slow_init(struct ucc_slow_info * us_info, struct > ucc_slow_private ** ucc > /* Get PRAM base */ > uccs->us_pram_offset = > qe_muram_alloc(UCC_SLOW_PRAM_SIZE, ALIGNMENT_OF_UCC_SLOW_PRAM); > - if (IS_ERR_VALUE(uccs->us_pram_offset)) { > + if (uccs->us_pram_offset < 0) { > printk(KERN_ERR "%s: cannot allocate MURAM for PRAM", __func__); > ucc_slow_free(uccs); > return -ENOMEM; > @@ -206,10 +209,9 @@ int ucc_slow_init(struct ucc_slow_info * us_info, struct > ucc_slow_private ** ucc > uccs->rx_base_offset = > qe_muram_alloc(us_info->rx_bd_ring_len * sizeof(struct qe_bd), > QE_ALIGNMENT_OF_BD); > - if (IS_ERR_VALUE(uccs->rx_base_offset)) { > + if (uccs->rx_base_offset < 0) { > printk(KERN_ERR "%s: cannot allocate %u RX BDs\n", __func__, > us_info->rx_bd_ring_len); > - uccs->rx_base_offset = 0; > ucc_slow_free(uccs); > return -ENOMEM; > } > @@ -217,9 +219,8 @@ int ucc_slow_init(struct ucc_slow_info * us_info, struct > ucc_slow_private ** ucc > uccs->tx_base_offset = > qe_muram_alloc(us_info->tx_bd_ring_len * sizeof(struct qe_bd), > QE_ALIGNMENT_OF_BD); > - if (IS_ERR_VALUE(uccs->tx_base_offset)) { > + if (uccs->tx_base_offset < 0) { > printk(KERN_ERR "%s: cannot allocate TX BDs", __func__); > - uccs->tx_base_offset = 0; > ucc_slow_free(uccs); > return -ENOMEM; > } > @@ -352,14 +353,9 @@ void ucc_slow_free(struct ucc_slow_private * uccs) > if (!uccs) > return; > > - if (uccs->rx_base_offset) > - qe_muram_free(uccs->rx_base_offset); > - > - if (uccs->tx_base_offset) > - qe_muram_free(uccs->tx_base_offset); > - > - if (uccs->us_pram) > - qe_muram_free(uccs->us_pram_offset); > + qe_muram_free(uccs->rx_base_offset); > + qe_muram_free(uccs->tx_base_offset); > + qe_muram_free(uccs->us_pram_offset); > > if (uccs->us_regs) > iounmap(uccs->us_regs); > diff --git a/include/soc/fsl/qe/ucc_slow.h b/include/soc/fsl/qe/ucc_slow.h > index 8696fdea2ae9..d187a6be83bc 100644 > --- a/include/soc/fsl/qe/ucc_slow.h > +++ b/include/soc/fsl/qe/ucc_slow.h > @@ -185,7 +185,7 @@ struct ucc_slow_private { > struct ucc_slow_info *us_info; > struct ucc_slow __iomem *us_regs; /* Ptr to memory map of UCC regs */ > struct ucc_slow_pram *us_pram; /* a pointer to the parameter RAM */ > - u32 us_pram_offset; > + s32 us_pram_offset; > int enabled_tx; /* Whether channel is enabled for Tx (ENT) */ > int enabled_rx; /* Whether channel is enabled for Rx (ENR) */ > int stopped_tx; /* Whether channel has been stopped for Tx > @@ -194,8 +194,8 @@ struct ucc_slow_private { > struct list_head confQ; /* frames passed to chip waiting for tx */ > u32 first_tx_bd_mask; /* mask is used in Tx routine to save status > and length for first BD in a frame */ > - u32 tx_base_offset; /* first BD in Tx BD table offset (In MURAM) */ > - u32 rx_base_offset; /* first BD in Rx BD table offset (In MURAM) */ > + s32 tx_base_offset; /* first BD in Tx BD table offset (In MURAM) */ > + s32 rx_base_offset; /* first BD in Rx BD table offset (In MURAM) */ > struct qe_bd *confBd; /* next BD for confirm after Tx */ > struct qe_bd *tx_bd; /* next BD for new Tx request */ > struct qe_bd *rx_bd; /* next BD to collect after Rx */ > -- > 2.23.0 > > > > ------------------------------ > > Subject: Digest Footer > > _______________________________________________ > Linuxppc-dev mailing list > Linuxppc-dev@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/linuxppc-dev > > > ------------------------------ > > End of Linuxppc-dev Digest, Vol 183, Issue 481 > **********************************************
