Re: [PATCH v1 2/4] soc/fsl/guts: Add definition for LX2160A

[Li Yang]

On Tue, Feb 26, 2019 at 4:12 AM Vabhav Sharma  wrote:
>
> Adding compatible string "lx2160a-dcfg" to
> initialize guts driver for lx2160 and SoC die
> attribute definition for LX2160A


Applied to branch next.  Thanks.

Regards,
Leo
>
> Signed-off-by: Vabhav Sharma 
> Signed-off-by: Yinbo Zhu 
> Acked-by: Li Yang 
> ---
>  drivers/soc/fsl/guts.c | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
> index 302e0c8..bcab1ee 100644
> --- a/drivers/soc/fsl/guts.c
> +++ b/drivers/soc/fsl/guts.c
> @@ -100,6 +100,11 @@ static const struct fsl_soc_die_attr fsl_soc_die[] = {
>   .svr  = 0x8700,
>   .mask = 0xfff7,
> },
> +   /* Die: LX2160A, SoC: LX2160A/LX2120A/LX2080A */
> +   { .die  = "LX2160A",
> + .svr  = 0x8736,
> + .mask = 0xff3f,
> +   },
> { },
>  };
>
> @@ -222,6 +227,7 @@ static const struct of_device_id fsl_guts_of_match[] = {
> { .compatible = "fsl,ls1088a-dcfg", },
> { .compatible = "fsl,ls1012a-dcfg", },
> { .compatible = "fsl,ls1046a-dcfg", },
> +   { .compatible = "fsl,lx2160a-dcfg", },
> {}
>  };
>  MODULE_DEVICE_TABLE(of, fsl_guts_of_match);
> --
> 2.7.4
>

Re: [PATCH] soc/fsl/qe: Fix an error code in qe_pin_request()

[Li Yang]

On Thu, Mar 28, 2019 at 9:21 AM Dan Carpenter  wrote:
>
> We forgot to set "err" on this error path.
>
> Fixes: 1a2d397a6eb5 ("gpio/powerpc: Eliminate duplication of 
> of_get_named_gpio_flags()")
> Signed-off-by: Dan Carpenter 

Applied to fix branch.  Thanks.

Regards,
Leo
> ---
>  drivers/soc/fsl/qe/gpio.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/fsl/qe/gpio.c b/drivers/soc/fsl/qe/gpio.c
> index 819bed0f5667..51b3a47b5a55 100644
> --- a/drivers/soc/fsl/qe/gpio.c
> +++ b/drivers/soc/fsl/qe/gpio.c
> @@ -179,8 +179,10 @@ struct qe_pin *qe_pin_request(struct device_node *np, 
> int index)
> if (err < 0)
> goto err0;
> gc = gpio_to_chip(err);
> -   if (WARN_ON(!gc))
> +   if (WARN_ON(!gc)) {
> +   err = -ENODEV;
> goto err0;
> +   }
>
> if (!of_device_is_compatible(gc->of_node, 
> "fsl,mpc8323-qe-pario-bank")) {
> pr_debug("%s: tried to get a non-qe pin\n", __func__);
> --
> 2.17.1
>

Re: [PATCH 02/13] soc/fsl/bman: map FBPR area in the iommu

[Li Yang]

On Fri, Mar 29, 2019 at 9:03 AM  wrote:
>
> From: Laurentiu Tudor 
>
> Add a one-to-one iommu mapping for bman private data memory (FBPR).
> This is required for BMAN to work without faults behind an iommu.
>
> Signed-off-by: Laurentiu Tudor 
> ---
>  drivers/soc/fsl/qbman/bman_ccsr.c | 11 +++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/soc/fsl/qbman/bman_ccsr.c 
> b/drivers/soc/fsl/qbman/bman_ccsr.c
> index 7c3cc968053c..b209c79511bb 100644
> --- a/drivers/soc/fsl/qbman/bman_ccsr.c
> +++ b/drivers/soc/fsl/qbman/bman_ccsr.c
> @@ -29,6 +29,7 @@
>   */
>
>  #include "bman_priv.h"
> +#include 
>
>  u16 bman_ip_rev;
>  EXPORT_SYMBOL(bman_ip_rev);
> @@ -178,6 +179,7 @@ static int fsl_bman_probe(struct platform_device *pdev)
> int ret, err_irq;
> struct device *dev = >dev;
> struct device_node *node = dev->of_node;
> +   struct iommu_domain *domain;
> struct resource *res;
> u16 id, bm_pool_cnt;
> u8 major, minor;
> @@ -225,6 +227,15 @@ static int fsl_bman_probe(struct platform_device *pdev)
>
> dev_dbg(dev, "Allocated FBPR 0x%llx 0x%zx\n", fbpr_a, fbpr_sz);
>
> +   /* Create an 1-to-1 iommu mapping for FBPR area */
> +   domain = iommu_get_domain_for_dev(dev);
> +   if (domain) {
> +   ret = iommu_map(domain, fbpr_a, fbpr_a, fbpr_sz,
> +   IOMMU_READ | IOMMU_WRITE | IOMMU_CACHE);
> +   if (ret)
> +   dev_warn(dev, "failed to iommu_map() %d\n", ret);
> +   }

Like Robin has pointed out, could you explain why the mapping in this
patch and other similar patches cannot be dealt with the dma APIs
automatically?  If the current bqman driver doesn't use the dma APIs
correctly, we need to fix that instead of doing the mapping
explicitly.

> +
> bm_set_memory(fbpr_a, fbpr_sz);
>
> err_irq = platform_get_irq(pdev, 0);
> --
> 2.17.1
>

Re: [PATCH 01/13] soc/fsl/qman: fixup liodns only on ppc targets

[Li Yang]

On Fri, Mar 29, 2019 at 9:01 AM  wrote:
>
> From: Laurentiu Tudor 
>
> ARM SoCs use SMMU so the liodn fixup done in the qman driver is no
> longer making sense and it also breaks the ICID settings inherited
> from u-boot. Do the fixups only for PPC targets.
>
> Signed-off-by: Laurentiu Tudor 
> ---
>  drivers/soc/fsl/qbman/qman_ccsr.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/soc/fsl/qbman/qman_ccsr.c 
> b/drivers/soc/fsl/qbman/qman_ccsr.c
> index 109b38de3176..12e414ca3b03 100644
> --- a/drivers/soc/fsl/qbman/qman_ccsr.c
> +++ b/drivers/soc/fsl/qbman/qman_ccsr.c
> @@ -598,6 +598,7 @@ static int qman_init_ccsr(struct device *dev)
>  #define LIO_CFG_LIODN_MASK 0x0fff
>  void qman_liodn_fixup(u16 channel)
>  {
> +#ifdef CONFIG_PPC
> static int done;
> static u32 liodn_offset;
> u32 before, after;
> @@ -617,6 +618,7 @@ void qman_liodn_fixup(u16 channel)
> qm_ccsr_out(REG_REV3_QCSP_LIO_CFG(idx), after);
> else
> qm_ccsr_out(REG_QCSP_LIO_CFG(idx), after);
> +#endif

According to the Linux coding style recommendation, it would be better
to put the #ifdef into the header files
"drivers/soc/fsl/qbman/qman_priv.h".  And I'm not sure if this is
needed on PPC when IOMMU(PAMU) driver is not compiled, if not,
probably using CONFIG_PAMU as condition would be even better.

>  }
>
>  #define IO_CFG_SDEST_MASK 0x00ff
> --
> 2.17.1
>

Re: [PATCH 05/13] soc/fsl/bqman: page align iommu mapping sizes

[Li Yang]

On Fri, Mar 29, 2019 at 9:01 AM  wrote:
>
> From: Laurentiu Tudor 
>
> Prior to calling iommu_map()/iommu_unmap() page align the size or
> failures such as below could happen:
>
> iommu: unaligned: iova 0x... pa 0x... size 0x4000 min_pagesz 0x1
> qman_portal 5.qman-portal: failed to iommu_map() -22
>
> Seen when booted a kernel compiled with 64K page size support.

This will silently incease the actual space mapped to 64K when the
driver is actually trying to map 4K.  Will this potentially cause
security breaches?  If it is really safe to map 64K, probably the
better way is to increase the region size to 64k in the device tree
explicitly.

>
> Signed-off-by: Laurentiu Tudor 
> ---
>  drivers/soc/fsl/qbman/bman_ccsr.c   | 2 +-
>  drivers/soc/fsl/qbman/qman_ccsr.c   | 4 ++--
>  drivers/soc/fsl/qbman/qman_portal.c | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/soc/fsl/qbman/bman_ccsr.c 
> b/drivers/soc/fsl/qbman/bman_ccsr.c
> index b209c79511bb..3a6e01bde32d 100644
> --- a/drivers/soc/fsl/qbman/bman_ccsr.c
> +++ b/drivers/soc/fsl/qbman/bman_ccsr.c
> @@ -230,7 +230,7 @@ static int fsl_bman_probe(struct platform_device *pdev)
> /* Create an 1-to-1 iommu mapping for FBPR area */
> domain = iommu_get_domain_for_dev(dev);
> if (domain) {
> -   ret = iommu_map(domain, fbpr_a, fbpr_a, fbpr_sz,
> +   ret = iommu_map(domain, fbpr_a, fbpr_a, PAGE_ALIGN(fbpr_sz),
> IOMMU_READ | IOMMU_WRITE | IOMMU_CACHE);
> if (ret)
> dev_warn(dev, "failed to iommu_map() %d\n", ret);
> diff --git a/drivers/soc/fsl/qbman/qman_ccsr.c 
> b/drivers/soc/fsl/qbman/qman_ccsr.c
> index eec7700507e1..8d3c950ce52d 100644
> --- a/drivers/soc/fsl/qbman/qman_ccsr.c
> +++ b/drivers/soc/fsl/qbman/qman_ccsr.c
> @@ -783,11 +783,11 @@ static int fsl_qman_probe(struct platform_device *pdev)
> /* Create an 1-to-1 iommu mapping for fqd and pfdr areas */
> domain = iommu_get_domain_for_dev(dev);
> if (domain) {
> -   ret = iommu_map(domain, fqd_a, fqd_a, fqd_sz,
> +   ret = iommu_map(domain, fqd_a, fqd_a, PAGE_ALIGN(fqd_sz),
> IOMMU_READ | IOMMU_WRITE | IOMMU_CACHE);
> if (ret)
> dev_warn(dev, "iommu_map(fqd) failed %d\n", ret);
> -   ret = iommu_map(domain, pfdr_a, pfdr_a, pfdr_sz,
> +   ret = iommu_map(domain, pfdr_a, pfdr_a, PAGE_ALIGN(pfdr_sz),
> IOMMU_READ | IOMMU_WRITE | IOMMU_CACHE);
> if (ret)
> dev_warn(dev, "iommu_map(pfdr) failed %d\n", ret);
> diff --git a/drivers/soc/fsl/qbman/qman_portal.c 
> b/drivers/soc/fsl/qbman/qman_portal.c
> index dfb62f9815e9..bce56da2b01f 100644
> --- a/drivers/soc/fsl/qbman/qman_portal.c
> +++ b/drivers/soc/fsl/qbman/qman_portal.c
> @@ -297,7 +297,7 @@ static int qman_portal_probe(struct platform_device *pdev)
>  */
> err = iommu_map(domain,
> addr_phys[0]->start, addr_phys[0]->start,
> -   resource_size(addr_phys[0]),
> +   PAGE_ALIGN(resource_size(addr_phys[0])),
> IOMMU_READ | IOMMU_WRITE | IOMMU_CACHE);
> if (err)
> dev_warn(dev, "failed to iommu_map() %d\n", err);
> --
> 2.17.1
>

Re: [PATCH 06/13] soc/fsl/qbman_portals: add APIs to retrieve the probing status

[Li Yang]

On Fri, Mar 29, 2019 at 9:03 AM  wrote:
>
> From: Laurentiu Tudor 
>
> Add a couple of new APIs to check the probing status of the required
> cpu bound qman and bman portals:
>  'int bman_portals_probed()' and 'int qman_portals_probed()'.
> They return the following values.
>  *  1 if qman/bman portals were all probed correctly
>  *  0 if qman/bman portals were not yet probed
>  * -1 if probing of qman/bman portals failed
> Portals are considered successful probed if no error occurred during
> the probing of any of the portals and if enough portals were probed
> to have one available for each cpu.
> The error handling paths were slightly rearranged in order to fit this
> new functionality without being too intrusive.
> Drivers that use qman/bman portal driver services are required to use
> these APIs before calling any functions exported by these drivers or
> otherwise they will crash the kernel.
> First user will be the dpaa1 ethernet driver, coming in a subsequent
> patch.
>
> Signed-off-by: Laurentiu Tudor 
> ---
>  drivers/soc/fsl/qbman/bman_portal.c | 22 ++
>  drivers/soc/fsl/qbman/qman_portal.c | 23 +++
>  include/soc/fsl/bman.h  |  8 
>  include/soc/fsl/qman.h  |  9 +
>  4 files changed, 54 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/soc/fsl/qbman/bman_portal.c 
> b/drivers/soc/fsl/qbman/bman_portal.c
> index 2c95cf59f3e7..7819bc29936d 100644
> --- a/drivers/soc/fsl/qbman/bman_portal.c
> +++ b/drivers/soc/fsl/qbman/bman_portal.c
> @@ -32,6 +32,7 @@
>
>  static struct bman_portal *affine_bportals[NR_CPUS];
>  static struct cpumask portal_cpus;
> +static int __bman_portals_probed;
>  /* protect bman global registers and global data shared among portals */
>  static DEFINE_SPINLOCK(bman_lock);
>
> @@ -87,6 +88,12 @@ static int bman_online_cpu(unsigned int cpu)
> return 0;
>  }
>
> +int bman_portals_probed(void)
> +{
> +   return __bman_portals_probed;
> +}
> +EXPORT_SYMBOL_GPL(bman_portals_probed);
> +
>  static int bman_portal_probe(struct platform_device *pdev)
>  {
> struct device *dev = >dev;
> @@ -104,8 +111,10 @@ static int bman_portal_probe(struct platform_device 
> *pdev)
> }
>
> pcfg = devm_kmalloc(dev, sizeof(*pcfg), GFP_KERNEL);
> -   if (!pcfg)
> +   if (!pcfg) {
> +   __bman_portals_probed = -1;
> return -ENOMEM;
> +   }
>
> pcfg->dev = dev;
>
> @@ -113,14 +122,14 @@ static int bman_portal_probe(struct platform_device 
> *pdev)
>  DPAA_PORTAL_CE);
> if (!addr_phys[0]) {
> dev_err(dev, "Can't get %pOF property 'reg::CE'\n", node);
> -   return -ENXIO;
> +   goto err_ioremap1;
> }
>
> addr_phys[1] = platform_get_resource(pdev, IORESOURCE_MEM,
>  DPAA_PORTAL_CI);
> if (!addr_phys[1]) {
> dev_err(dev, "Can't get %pOF property 'reg::CI'\n", node);
> -   return -ENXIO;
> +   goto err_ioremap1;
> }
>
> pcfg->cpu = -1;
> @@ -128,7 +137,7 @@ static int bman_portal_probe(struct platform_device *pdev)
> irq = platform_get_irq(pdev, 0);
> if (irq <= 0) {
> dev_err(dev, "Can't get %pOF IRQ'\n", node);
> -   return -ENXIO;
> +   goto err_ioremap1;
> }
> pcfg->irq = irq;
>
> @@ -156,6 +165,9 @@ static int bman_portal_probe(struct platform_device *pdev)
> }
>
> cpumask_set_cpu(cpu, _cpus);
> +   if (!__bman_portals_probed &&
> +   cpumask_weight(_cpus) == num_online_cpus())
> +   __bman_portals_probed = 1;

Given the fact that the portal_cpus bit will get set even for offline
cpus, this is not correct either.  Probably the previous code for
checking cpu >= nr_cpu_ids is actually the right way to do it.

> spin_unlock(_lock);
> pcfg->cpu = cpu;
>
> @@ -175,6 +187,8 @@ static int bman_portal_probe(struct platform_device *pdev)
>  err_ioremap2:
> memunmap(pcfg->addr_virt_ce);
>  err_ioremap1:
> +__bman_portals_probed = -1;
> +
> return -ENXIO;
>  }
>
> diff --git a/drivers/soc/fsl/qbman/qman_portal.c 
> b/drivers/soc/fsl/qbman/qman_portal.c
> index bce56da2b01f..11ba6c77c0d6 100644
> --- a/drivers/soc/fsl/qbman/qman_portal.c
> +++ b/drivers/soc/fsl/qbman/qman_portal.c
> @@ -39,6 +39,7 @@ EXPORT_SYMBOL(qman_dma_portal);
>  #define CONFIG_FSL_DPA_PIRQ_FAST  1
>
>  static struct cpumask portal_cpus;
> +static int __qman_portals_probed;
>  /* protect qman global registers and global data shared among portals */
>  static DEFINE_SPINLOCK(qman_lock);
>
> @@ -221,6 +222,12 @@ static int qman_online_cpu(unsigned int cpu)
> return 0;
>  }
>
> +int qman_portals_probed(void)
> +{
> +   return __qman_portals_probed;
> +}
> +EXPORT_SYMBOL_GPL(qman_portals_probed);
> +
>  

Re: [PATCH] soc: fsl: guts: make fsl_guts_get_svr() static

[Li Yang]

On Fri, Feb 22, 2019 at 2:09 AM Y.b. Lu  wrote:
>
>
>
> > -Original Message-
> > From: Horia Geantă 
> > Sent: Thursday, February 21, 2019 6:38 PM
> > To: Leo Li 
> > Cc: Y.b. Lu ; linuxppc-dev@lists.ozlabs.org;
> > linux-arm-ker...@lists.infradead.org; linux-ker...@vger.kernel.org
> > Subject: [PATCH] soc: fsl: guts: make fsl_guts_get_svr() static
> >
> > The export of fsl_guts_get_svr() is a left-over, it's currently used only 
> > internally
> > and users needing SoC information should use the generic soc_device
> > infrastructure.
> >
>
> [Y.b. Lu] Acked-by: Yangbo Lu 

Applied for next.  Thanks.

>
> > Signed-off-by: Horia Geantă 
> > ---
> >  drivers/soc/fsl/guts.c   | 3 +--
> >  include/linux/fsl/guts.h | 2 --
> >  2 files changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c index
> > 4f9655087bd7..63f6df86f9e5 100644
> > --- a/drivers/soc/fsl/guts.c
> > +++ b/drivers/soc/fsl/guts.c
> > @@ -115,7 +115,7 @@ static const struct fsl_soc_die_attr
> > *fsl_soc_die_match(
> >   return NULL;
> >  }
> >
> > -u32 fsl_guts_get_svr(void)
> > +static u32 fsl_guts_get_svr(void)
> >  {
> >   u32 svr = 0;
> >
> > @@ -129,7 +129,6 @@ u32 fsl_guts_get_svr(void)
> >
> >   return svr;
> >  }
> > -EXPORT_SYMBOL(fsl_guts_get_svr);
> >
> >  static int fsl_guts_probe(struct platform_device *pdev)  { diff --git
> > a/include/linux/fsl/guts.h b/include/linux/fsl/guts.h index
> > 941b11811f85..1fc0edd71c52 100644
> > --- a/include/linux/fsl/guts.h
> > +++ b/include/linux/fsl/guts.h
> > @@ -135,8 +135,6 @@ struct ccsr_guts {
> >   u32 srds2cr1;   /* 0x.0f44 - SerDes2 Control Register 0 */
> >  } __attribute__ ((packed));
> >
> > -u32 fsl_guts_get_svr(void);
> > -
> >  /* Alternate function signal multiplex control */  #define
> > MPC85xx_PMUXCR_QE(x) (0x8000 >> (x))
> >
> > --
> > 2.16.2
>

Re: [PATCH] ucc_geth: Reset BQL queue when stopping device

[Li Yang]

On Tue, Jan 29, 2019 at 2:09 AM Mathias Thore
 wrote:
>
> Is there a scenario where we are clearing the TX ring but don't want to reset 
> the BQL TX queue?

Right now the function is also used on interface open/close, driver
removal and driver resumption besides the timeout situation. I think
the reseting BQL queue is either not neccessary or already called
explicitly for the other scenarios.

>
> I think it makes sense to keep it in ucc_geth_free_tx since the reason it is 
> needed isn't the timeout per se, but rather the clearing of the TX ring. This 
> way, it will be performed no matter why the driver ends up calling this 
> function.

I don't see a consensus on when netdev_reset_queue() should be called
among existing drivers.  Doing it on buffer ring cleanup probably can
be future-proofing.  But if we want to use this approach, we can
remove the redundent netdev_reset_queue() calls in open/close
functions.

Regards,
Leo

>
> -Original Message-
> From: Li Yang [mailto:leoyang...@nxp.com]
> Sent: Monday, 28 January 2019 22:37
> To: Mathias Thore 
> Cc: Christophe Leroy ; net...@vger.kernel.org; 
> linuxppc-dev@lists.ozlabs.org; David Gounaris ; 
> Joakim Tjernlund 
> Subject: Re: [PATCH] ucc_geth: Reset BQL queue when stopping device
>
> On Mon, Jan 28, 2019 at 8:37 AM Mathias Thore  
> wrote:
> >
> > Hi,
> >
> >
> > This is what we observed: there was a storm on the medium so that our 
> > controller could not do its TX, resulting in timeout. When timeout occurs, 
> > the driver clears all descriptors from the TX queue. The function called in 
> > this patch is used to reflect this clearing also in the BQL layer. Without 
> > it, the controller would get stuck, unable to perform TX, even several 
> > minutes after the storm had ended. Bringing the device down and then up 
> > again would solve the problem, but this patch also solves it automatically.
>
> The explanation makes sense.  So this should only be required in the timeout 
> scenario instead of other clean up scenarios like device shutdown?  If so, it 
> probably it will be better to be done in ucc_geth_timeout_work()?
>
> >
> >
> > Some other drivers do the same, for example e1000e driver calls 
> > netdev_reset_queue in its e1000_clean_tx_ring function. It is possible that 
> > other drivers should do the same; I have no way of verifying this.
> >
> >
> > Regards,
> >
> > Mathias
> >
> > --
> >
> >
> > From: Christophe Leroy 
> > Sent: Monday, January 28, 2019 10:48 AM
> > To: Mathias Thore; leoyang...@nxp.com; net...@vger.kernel.org;
> > linuxppc-dev@lists.ozlabs.org; David Gounaris; Joakim Tjernlund
> > Subject: Re: [PATCH] ucc_geth: Reset BQL queue when stopping device
> >
> >
> > CAUTION: This email originated from outside of the organization. Do not 
> > click links or open attachments unless you recognize the sender and know 
> > the content is safe.
> >
> >
> > Hi,
> >
> > Le 28/01/2019 à 10:07, Mathias Thore a écrit :
> > > After a timeout event caused by for example a broadcast storm, when
> > > the MAC and PHY are reset, the BQL TX queue needs to be reset as
> > > well. Otherwise, the device will exhibit severe performance issues
> > > even after the storm has ended.
> >
> > What are the symptomns ?
> >
> > Is this reset needed on any network driver in that case, or is it
> > something particular for the ucc_geth ?
> > For instance, the freescale fs_enet doesn't have that reset. Should it
> > have it too ?
> >
> > Christophe
> >
> > >
> > > Co-authored-by: David Gounaris 
> > > Signed-off-by: Mathias Thore 
> > > ---
> > >   drivers/net/ethernet/freescale/ucc_geth.c | 2 ++
> > >   1 file changed, 2 insertions(+)
> > >
> > > diff --git a/drivers/net/ethernet/freescale/ucc_geth.c
> > > b/drivers/net/ethernet/freescale/ucc_geth.c
> > > index c3d539e209ed..eb3e65e8868f 100644
> > > --- a/drivers/net/ethernet/freescale/ucc_geth.c
> > > +++ b/drivers/net/ethernet/freescale/ucc_geth.c
> > > @@ -1879,6 +1879,8 @@ static void ucc_geth_free_tx(struct 
> > > ucc_geth_private *ugeth)
> > >   u16 i, j;
> > >   u8 __iomem *bd;
> > >
> > > + netdev_reset_queue(ugeth->ndev);
> > > +
> > >   ug_info = ugeth->ug_info;
> > >   uf_info = _info->uf_info;
> > >
> > >
> >

Re: [PATCH] ucc_geth: Reset BQL queue when stopping device

[Li Yang]

On Mon, Jan 28, 2019 at 8:37 AM Mathias Thore
 wrote:
>
> Hi,
>
>
> This is what we observed: there was a storm on the medium so that our 
> controller could not do its TX, resulting in timeout. When timeout occurs, 
> the driver clears all descriptors from the TX queue. The function called in 
> this patch is used to reflect this clearing also in the BQL layer. Without 
> it, the controller would get stuck, unable to perform TX, even several 
> minutes after the storm had ended. Bringing the device down and then up again 
> would solve the problem, but this patch also solves it automatically.

The explanation makes sense.  So this should only be required in the
timeout scenario instead of other clean up scenarios like device
shutdown?  If so, it probably it will be better to be done in
ucc_geth_timeout_work()?

>
>
> Some other drivers do the same, for example e1000e driver calls 
> netdev_reset_queue in its e1000_clean_tx_ring function. It is possible that 
> other drivers should do the same; I have no way of verifying this.
>
>
> Regards,
>
> Mathias
>
> --
>
>
> From: Christophe Leroy 
> Sent: Monday, January 28, 2019 10:48 AM
> To: Mathias Thore; leoyang...@nxp.com; net...@vger.kernel.org; 
> linuxppc-dev@lists.ozlabs.org; David Gounaris; Joakim Tjernlund
> Subject: Re: [PATCH] ucc_geth: Reset BQL queue when stopping device
>
>
> CAUTION: This email originated from outside of the organization. Do not click 
> links or open attachments unless you recognize the sender and know the 
> content is safe.
>
>
> Hi,
>
> Le 28/01/2019 à 10:07, Mathias Thore a écrit :
> > After a timeout event caused by for example a broadcast storm, when
> > the MAC and PHY are reset, the BQL TX queue needs to be reset as
> > well. Otherwise, the device will exhibit severe performance issues
> > even after the storm has ended.
>
> What are the symptomns ?
>
> Is this reset needed on any network driver in that case, or is it
> something particular for the ucc_geth ?
> For instance, the freescale fs_enet doesn't have that reset. Should it
> have it too ?
>
> Christophe
>
> >
> > Co-authored-by: David Gounaris 
> > Signed-off-by: Mathias Thore 
> > ---
> >   drivers/net/ethernet/freescale/ucc_geth.c | 2 ++
> >   1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/freescale/ucc_geth.c 
> > b/drivers/net/ethernet/freescale/ucc_geth.c
> > index c3d539e209ed..eb3e65e8868f 100644
> > --- a/drivers/net/ethernet/freescale/ucc_geth.c
> > +++ b/drivers/net/ethernet/freescale/ucc_geth.c
> > @@ -1879,6 +1879,8 @@ static void ucc_geth_free_tx(struct ucc_geth_private 
> > *ugeth)
> >   u16 i, j;
> >   u8 __iomem *bd;
> >
> > + netdev_reset_queue(ugeth->ndev);
> > +
> >   ug_info = ugeth->ug_info;
> >   uf_info = _info->uf_info;
> >
> >
>

Re: [PATCH] soc: fsl: dpio: Use after free in dpaa2_dpio_remove()

[Li Yang]

On Mon, Feb 4, 2019 at 8:12 AM Dan Carpenter  wrote:
>
> The dpaa2_io_down(priv->io) call frees "priv->io" so I've shifted the
> code around a little bit to avoid the use after free.
>
> Fixes: 991e873223e9 ("soc: fsl: dpio: use a cpumask to identify which cpus 
> are unused")
> Signed-off-by: Dan Carpenter 

Applied.  Thanks.

> ---
>  drivers/soc/fsl/dpio/dpio-driver.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/soc/fsl/dpio/dpio-driver.c 
> b/drivers/soc/fsl/dpio/dpio-driver.c
> index 2d4af32a0dec..a28799b62d53 100644
> --- a/drivers/soc/fsl/dpio/dpio-driver.c
> +++ b/drivers/soc/fsl/dpio/dpio-driver.c
> @@ -220,12 +220,12 @@ static int dpaa2_dpio_remove(struct fsl_mc_device 
> *dpio_dev)
>
> dev = _dev->dev;
> priv = dev_get_drvdata(dev);
> +   cpu = dpaa2_io_get_cpu(priv->io);
>
> dpaa2_io_down(priv->io);
>
> dpio_teardown_irqs(dpio_dev);
>
> -   cpu = dpaa2_io_get_cpu(priv->io);
> cpumask_set_cpu(cpu, cpus_unused_mask);
>
> err = dpio_open(dpio_dev->mc_io, 0, dpio_dev->obj_desc.id,
> --
> 2.17.1
>

Re: [PATCH v3 1/2] dt-bindings: soc: fsl: Document Qixis FPGA usage

[Li Yang]

Please include device tree binding mailing list and maintainers for
binding patches(cc'ed now).

On Mon, Feb 4, 2019 at 3:15 AM Pankaj Bansal  wrote:
>
> an FPGA-based system controller, called “Qixis”, which
> manages several critical system features, including:
> • Reset sequencing
> • Power supply configuration
> • Board configuration
> • hardware configuration
>
> The qixis registers are accessible over one or more system-specific
> interfaces, typically I2C, JTAG or an embedded processor.
>
> Signed-off-by: Pankaj Bansal 
> ---
>
> Notes:
> V3:
> - Added boardname based compatible field in bindings
> - Added bindings for MMIO based FPGA
> V2:
> - No change
>
>  .../bindings/soc/fsl/qixis_ctrl.txt  | 53 ++
>  1 file changed, 53 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/soc/fsl/qixis_ctrl.txt 
> b/Documentation/devicetree/bindings/soc/fsl/qixis_ctrl.txt
> new file mode 100644
> index ..5d510df14be8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/fsl/qixis_ctrl.txt
> @@ -0,0 +1,53 @@
> +* QIXIS FPGA block
> +
> +an FPGA-based system controller, called “Qixis”, which
> +manages several critical system features, including:
> +• Configuration switch monitoring
> +• Power on/off sequencing
> +• Reset sequencing
> +• Power supply configuration
> +• Board configuration
> +• hardware configuration
> +• Background power data collection (DCM)
> +• Fault monitoring
> +• RCW bypass SRAM (replace flash RCW with internal RCW) (NOR only)
> +• Dedicated functional validation blocks (POSt/IRS, triggered event, and so 
> on)
> +• I2C master for remote board control even with no DUT available
> +
> +The qixis registers are accessible over one or more system-specific 
> interfaces,
> +typically I2C, JTAG or an embedded processor.
> +
> +FPGA connected to I2C:
> +Required properties:
> +
> + - compatible: should be a board-specific string followed by a string
> +   indicating the type of FPGA.  Example:
> +   "fsl,-fpga", "fsl,fpga-qixis-i2c"
> + - reg : i2c address of the qixis device.
> +
> +Example (LX2160A-QDS):
> +   /* The FPGA node */
> +fpga@66 {
> +   compatible = "fsl,lx2160aqds-fpga", "fsl,fpga-qixis-i2c";
> +   reg = <0x66>;
> +   #address-cells = <1>;
> +   #size-cells = <0>;
> +   }
> +
> +* Freescale on-board FPGA
> +
> +This is the memory-mapped registers for on board FPGA.
> +
> +Required properties:
> +- compatible: should be a board-specific string followed by a string
> +  indicating the type of FPGA.  Example:
> +   "fsl,-fpga", "fsl,fpga-qixis"
> +- reg: should contain the address and the length of the FPGA register set.
> +
> +Example (LS2080A-RDB):
> +
> +cpld@3,0 {
> +compatible = "fsl,ls2080ardb-fpga", "fsl,fpga-qixis";
> +reg = <0x3 0 0x1>;
> +};
> +
> --
> 2.17.1
>

Re: [PATCH v3 0/6] soc/fsl/qe: cleanups and new DT binding

[Li Yang]

On Mon, May 13, 2019 at 6:17 AM Rasmus Villemoes
 wrote:
>
> This small series consists of some small cleanups and simplifications
> of the QUICC engine driver, and introduces a new DT binding that makes
> it much easier to support other variants of the QUICC engine IP block
> that appears in the wild: There's no reason to expect in general that
> the number of valid SNUMs uniquely determines the set of such, so it's
> better to simply let the device tree specify the values (and,
> implicitly via the array length, also the count).
>
> Which tree should this go through?
>
> v3:
> - Move example from commit log into binding document (adapting to
>   MPC8360 which the existing example pertains to).
> - Add more review tags.
> - Fix minor style issue.
>
> v2:
> - Address comments from Christophe Leroy
> - Add his Reviewed-by to 1/6 and 3/6
> - Split DT binding update to separate patch as per
>   Documentation/devicetree/bindings/submitting-patches.txt
>
> Rasmus Villemoes (6):
>   soc/fsl/qe: qe.c: drop useless static qualifier
>   soc/fsl/qe: qe.c: reduce static memory footprint by 1.7K
>   soc/fsl/qe: qe.c: introduce qe_get_device_node helper
>   dt-bindings: soc/fsl: qe: document new fsl,qe-snums binding
>   soc/fsl/qe: qe.c: support fsl,qe-snums property
>   soc/fsl/qe: qe.c: fold qe_get_num_of_snums into qe_snums_init

Series applied to soc/fsl for-next.  Thanks!

Regards,
Leo

>
>  .../devicetree/bindings/soc/fsl/cpm_qe/qe.txt |  13 +-
>  drivers/soc/fsl/qe/qe.c   | 163 +++---
>  2 files changed, 77 insertions(+), 99 deletions(-)
>
> --
> 2.20.1
>

Re: [PATCH v3 0/6] Prerequisites for NXP LS104xA SMMU enablement

[Li Yang]

On Thu, May 30, 2019 at 5:09 PM David Miller  wrote:
>
> From: laurentiu.tu...@nxp.com
> Date: Thu, 30 May 2019 17:19:45 +0300
>
> > Depends on this pull request:
> >
> >  http://lists.infradead.org/pipermail/linux-arm-kernel/2019-May/653554.html
>
> I'm not sure how you want me to handle this.

One suggestion from the arm-soc maintainers is that after this pull
request is merged by arm-soc tree.  You can also merge this pull
request and then apply the patches.

Regards,
Leo

Re: [PATCH][next] soc: fsl: fix spelling mistake "Firmaware" -> "Firmware"

[Li Yang]

On Tue, May 21, 2019 at 3:57 AM Colin King  wrote:
>
> From: Colin Ian King 
>
> There is a spelling mistake in a pr_err message. Fix it.
>
> Signed-off-by: Colin Ian King 

Applied.  Thanks!

Regards,
Leo
> ---
>  drivers/soc/fsl/dpaa2-console.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/soc/fsl/dpaa2-console.c b/drivers/soc/fsl/dpaa2-console.c
> index 9168d8ddc932..27243f706f37 100644
> --- a/drivers/soc/fsl/dpaa2-console.c
> +++ b/drivers/soc/fsl/dpaa2-console.c
> @@ -73,7 +73,7 @@ static u64 get_mc_fw_base_address(void)
>
> mcfbaregs = ioremap(mc_base_addr.start, resource_size(_base_addr));
> if (!mcfbaregs) {
> -   pr_err("could not map MC Firmaware Base registers\n");
> +   pr_err("could not map MC Firmware Base registers\n");
> return 0;
> }
>
> --
> 2.20.1
>

Re: [PATCH v2 0/2] soc: fsl: dpio: Add support for memory backed QBMan portals

[Li Yang]

On Fri, Apr 5, 2019 at 9:42 AM Roy Pledge  wrote:
>
> This patch series adds support for QBMan memory backed portals which is
> avaialble in devices containing QBMan verion 5.0 and above (for example
> NXP's LX2160A SoC).
>
> Memory backed portals can be mapped as normal cacheable/shareable memory
> which allows the portals to migrate between cores without needing manual
> cache manipulations by the CPU.
>
> The patches add support for the new portal attributes in the fsl-mc bus
> drivers as well as modifying the QBMan driver to use the new portal read
> trigger mechanism.
>
> Changes since v1:
>  * Support older DPRC command in case of older MC firmware
>  * Fix issue with padding in command
>
>
> Roy Pledge (2):
>   bus: mc-bus: Add support for mapping shareable portals
>   soc: fsl: dpio: Add support for memory backed QBMan portals

Both applied for next.  Thanks.

>
>  drivers/bus/fsl-mc/dprc.c   |  30 +++-
>  drivers/bus/fsl-mc/fsl-mc-bus.c |  15 +++-
>  drivers/bus/fsl-mc/fsl-mc-private.h |  17 -
>  drivers/soc/fsl/dpio/dpio-driver.c  |  23 --
>  drivers/soc/fsl/dpio/qbman-portal.c | 148 
> ++--
>  drivers/soc/fsl/dpio/qbman-portal.h |   5 ++
>  6 files changed, 199 insertions(+), 39 deletions(-)
>
> --
> 2.7.4
>

Re: [PATCH v2 1/9] soc/fsl/qman: fixup liodns only on ppc targets

[Li Yang]

On Sat, Apr 27, 2019 at 2:14 AM  wrote:
>
> From: Laurentiu Tudor 
>
> ARM SoCs use SMMU so the liodn fixup done in the qman driver is no
> longer making sense and it also breaks the ICID settings inherited
> from u-boot. Do the fixups only for PPC targets.
>
> Signed-off-by: Laurentiu Tudor 

Applied for next.  Thanks.

Leo
> ---
>  drivers/soc/fsl/qbman/qman_ccsr.c | 2 +-
>  drivers/soc/fsl/qbman/qman_priv.h | 9 -
>  2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/soc/fsl/qbman/qman_ccsr.c 
> b/drivers/soc/fsl/qbman/qman_ccsr.c
> index 109b38de3176..a6bb43007d03 100644
> --- a/drivers/soc/fsl/qbman/qman_ccsr.c
> +++ b/drivers/soc/fsl/qbman/qman_ccsr.c
> @@ -596,7 +596,7 @@ static int qman_init_ccsr(struct device *dev)
>  }
>
>  #define LIO_CFG_LIODN_MASK 0x0fff
> -void qman_liodn_fixup(u16 channel)
> +void __qman_liodn_fixup(u16 channel)
>  {
> static int done;
> static u32 liodn_offset;
> diff --git a/drivers/soc/fsl/qbman/qman_priv.h 
> b/drivers/soc/fsl/qbman/qman_priv.h
> index 75a8f905f8f7..04515718cfd9 100644
> --- a/drivers/soc/fsl/qbman/qman_priv.h
> +++ b/drivers/soc/fsl/qbman/qman_priv.h
> @@ -193,7 +193,14 @@ extern struct gen_pool *qm_cgralloc; /* CGR ID allocator 
> */
>  u32 qm_get_pools_sdqcr(void);
>
>  int qman_wq_alloc(void);
> -void qman_liodn_fixup(u16 channel);
> +#ifdef CONFIG_FSL_PAMU
> +#define qman_liodn_fixup __qman_liodn_fixup
> +#else
> +static inline void qman_liodn_fixup(u16 channel)
> +{
> +}
> +#endif
> +void __qman_liodn_fixup(u16 channel);
>  void qman_set_sdest(u16 channel, unsigned int cpu_idx);
>
>  struct qman_portal *qman_create_affine_portal(
> --
> 2.17.1
>

Re: [PATCH v2 2/9] soc/fsl/qbman_portals: add APIs to retrieve the probing status

[Li Yang]

On Sat, Apr 27, 2019 at 2:14 AM  wrote:
>
> From: Laurentiu Tudor 
>
> Add a couple of new APIs to check the probing status of the required
> cpu bound qman and bman portals:
>  'int bman_portals_probed()' and 'int qman_portals_probed()'.
> They return the following values.
>  *  1 if qman/bman portals were all probed correctly
>  *  0 if qman/bman portals were not yet probed
>  * -1 if probing of qman/bman portals failed
> Portals are considered successful probed if no error occurred during
> the probing of any of the portals and if enough portals were probed
> to have one available for each cpu.
> The error handling paths were slightly rearranged in order to fit this
> new functionality without being too intrusive.
> Drivers that use qman/bman portal driver services are required to use
> these APIs before calling any functions exported by these drivers or
> otherwise they will crash the kernel.
> First user will be the dpaa1 ethernet driver, coming in a subsequent
> patch.
>
> Signed-off-by: Laurentiu Tudor 

Applied for next.  Thanks.

Leo

> ---
>  drivers/soc/fsl/qbman/bman_portal.c | 20 
>  drivers/soc/fsl/qbman/qman_portal.c | 21 +
>  include/soc/fsl/bman.h  |  8 
>  include/soc/fsl/qman.h  |  9 +
>  4 files changed, 50 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/soc/fsl/qbman/bman_portal.c 
> b/drivers/soc/fsl/qbman/bman_portal.c
> index 2c95cf59f3e7..cf4f10d6f590 100644
> --- a/drivers/soc/fsl/qbman/bman_portal.c
> +++ b/drivers/soc/fsl/qbman/bman_portal.c
> @@ -32,6 +32,7 @@
>
>  static struct bman_portal *affine_bportals[NR_CPUS];
>  static struct cpumask portal_cpus;
> +static int __bman_portals_probed;
>  /* protect bman global registers and global data shared among portals */
>  static DEFINE_SPINLOCK(bman_lock);
>
> @@ -87,6 +88,12 @@ static int bman_online_cpu(unsigned int cpu)
> return 0;
>  }
>
> +int bman_portals_probed(void)
> +{
> +   return __bman_portals_probed;
> +}
> +EXPORT_SYMBOL_GPL(bman_portals_probed);
> +
>  static int bman_portal_probe(struct platform_device *pdev)
>  {
> struct device *dev = >dev;
> @@ -104,8 +111,10 @@ static int bman_portal_probe(struct platform_device 
> *pdev)
> }
>
> pcfg = devm_kmalloc(dev, sizeof(*pcfg), GFP_KERNEL);
> -   if (!pcfg)
> +   if (!pcfg) {
> +   __bman_portals_probed = -1;
> return -ENOMEM;
> +   }
>
> pcfg->dev = dev;
>
> @@ -113,14 +122,14 @@ static int bman_portal_probe(struct platform_device 
> *pdev)
>  DPAA_PORTAL_CE);
> if (!addr_phys[0]) {
> dev_err(dev, "Can't get %pOF property 'reg::CE'\n", node);
> -   return -ENXIO;
> +   goto err_ioremap1;
> }
>
> addr_phys[1] = platform_get_resource(pdev, IORESOURCE_MEM,
>  DPAA_PORTAL_CI);
> if (!addr_phys[1]) {
> dev_err(dev, "Can't get %pOF property 'reg::CI'\n", node);
> -   return -ENXIO;
> +   goto err_ioremap1;
> }
>
> pcfg->cpu = -1;
> @@ -128,7 +137,7 @@ static int bman_portal_probe(struct platform_device *pdev)
> irq = platform_get_irq(pdev, 0);
> if (irq <= 0) {
> dev_err(dev, "Can't get %pOF IRQ'\n", node);
> -   return -ENXIO;
> +   goto err_ioremap1;
> }
> pcfg->irq = irq;
>
> @@ -150,6 +159,7 @@ static int bman_portal_probe(struct platform_device *pdev)
> spin_lock(_lock);
> cpu = cpumask_next_zero(-1, _cpus);
> if (cpu >= nr_cpu_ids) {
> +   __bman_portals_probed = 1;
> /* unassigned portal, skip init */
> spin_unlock(_lock);
> return 0;
> @@ -175,6 +185,8 @@ static int bman_portal_probe(struct platform_device *pdev)
>  err_ioremap2:
> memunmap(pcfg->addr_virt_ce);
>  err_ioremap1:
> +__bman_portals_probed = -1;
> +
> return -ENXIO;
>  }
>
> diff --git a/drivers/soc/fsl/qbman/qman_portal.c 
> b/drivers/soc/fsl/qbman/qman_portal.c
> index 661c9b234d32..e2186b681d87 100644
> --- a/drivers/soc/fsl/qbman/qman_portal.c
> +++ b/drivers/soc/fsl/qbman/qman_portal.c
> @@ -38,6 +38,7 @@ EXPORT_SYMBOL(qman_dma_portal);
>  #define CONFIG_FSL_DPA_PIRQ_FAST  1
>
>  static struct cpumask portal_cpus;
> +static int __qman_portals_probed;
>  /* protect qman global registers and global data shared among portals */
>  static DEFINE_SPINLOCK(qman_lock);
>
> @@ -220,6 +221,12 @@ static int qman_online_cpu(unsigned int cpu)
> return 0;
>  }
>
> +int qman_portals_probed(void)
> +{
> +   return __qman_portals_probed;
> +}
> +EXPORT_SYMBOL_GPL(qman_portals_probed);
> +
>  static int qman_portal_probe(struct platform_device *pdev)
>  {
> struct device *dev = >dev;
> @@ -238,8 +245,10 @@ static int 

Re: [PATCH] soc: fsl: qe: gpio: Fix an error code in qe_pin_request()

[Li Yang]

On Fri, May 3, 2019 at 8:19 AM Dan Carpenter  wrote:
>
> There was a missing error code in this path.  It meant that we returned
> ERR_PTR(0) which is NULL and would result in a NULL dereference in the
> caller.

Thanks Dan.  An early version of this patch has been included in a
pending pull request to arm-soc.
https://lkml.org/lkml/2019/5/1/506


>
> Fixes: 1a2d397a6eb5 ("gpio/powerpc: Eliminate duplication of 
> of_get_named_gpio_flags()")
> Signed-off-by: Dan Carpenter 
> ---
>  drivers/soc/fsl/qe/gpio.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/fsl/qe/gpio.c b/drivers/soc/fsl/qe/gpio.c
> index 819bed0f5667..51b3a47b5a55 100644
> --- a/drivers/soc/fsl/qe/gpio.c
> +++ b/drivers/soc/fsl/qe/gpio.c
> @@ -179,8 +179,10 @@ struct qe_pin *qe_pin_request(struct device_node *np, 
> int index)
> if (err < 0)
> goto err0;
> gc = gpio_to_chip(err);
> -   if (WARN_ON(!gc))
> +   if (WARN_ON(!gc)) {
> +   err = -ENODEV;
> goto err0;
> +   }
>
> if (!of_device_is_compatible(gc->of_node, 
> "fsl,mpc8323-qe-pario-bank")) {
> pr_debug("%s: tried to get a non-qe pin\n", __func__);
> --
> 2.18.0
>

Re: [PATCH v3 0/7] soc/fsl/qbman: Enable Kexec for DPAA1 devices

[Li Yang]

On Thu, Aug 1, 2019 at 3:20 PM Roy Pledge  wrote:
>
> Most DPAA1 devices do not support a soft reset which is an issue if
> Kexec starts a new kernel. This patch series allows Kexec to function
> by detecting that the QBMan device was previously initialized.
>
> The patches fix some issues with device cleanup as well as ensuring
> that the location of the QBMan private memories has not changed
> after the execution of the Kexec.
>
> Changes since v1:
> - Removed a bug fix and sent it separately to ease backporting
> Changes since v2:
> - Expliciitly flush FQD memory from cache on PPC before unmapping
>
> Roy Pledge (7):
>   soc/fsl/qbman: Rework QBMan private memory setup
>   soc/fsl/qbman: Cleanup buffer pools if BMan was initialized prior to
> bootup
>   soc/fsl/qbman: Cleanup QMan queues if device was already initialized
>   soc/fsl/qbman: Fix drain_mr_fqni()
>   soc/fsl/qbman: Disable interrupts during portal recovery
>   soc/fsl/qbman: Fixup qman_shutdown_fq()
>   soc/fsl/qbman: Update device tree with reserved memory

Series applied for next.  Thanks!

>
>  drivers/soc/fsl/qbman/bman.c| 17 
>  drivers/soc/fsl/qbman/bman_ccsr.c   | 36 +++-
>  drivers/soc/fsl/qbman/bman_portal.c | 18 +++-
>  drivers/soc/fsl/qbman/bman_priv.h   |  5 +++
>  drivers/soc/fsl/qbman/dpaa_sys.c| 63 
>  drivers/soc/fsl/qbman/qman.c| 83 
> +
>  drivers/soc/fsl/qbman/qman_ccsr.c   | 68 +++---
>  drivers/soc/fsl/qbman/qman_portal.c | 18 +++-
>  drivers/soc/fsl/qbman/qman_priv.h   |  8 
>  9 files changed, 255 insertions(+), 61 deletions(-)
>
> --
> 2.7.4
>

Re: [PATCH v3 35/36] net/wan: make FSL_UCC_HDLC explicitly depend on PPC32

[Li Yang]

On Mon, Nov 4, 2019 at 2:39 AM Rasmus Villemoes
 wrote:
>
> On 01/11/2019 23.31, Leo Li wrote:
> >
> >
> >> -Original Message-
> >> From: Christophe Leroy 
> >> Sent: Friday, November 1, 2019 11:30 AM
> >> To: Rasmus Villemoes ; Qiang Zhao
> >> ; Leo Li 
> >> Cc: linuxppc-dev@lists.ozlabs.org; linux-arm-ker...@lists.infradead.org;
> >> linux-ker...@vger.kernel.org; Scott Wood ;
> >> net...@vger.kernel.org
> >> Subject: Re: [PATCH v3 35/36] net/wan: make FSL_UCC_HDLC explicitly
> >> depend on PPC32
> >>
> >>
> >>
> >> Le 01/11/2019 à 13:42, Rasmus Villemoes a écrit :
> >>> Currently, FSL_UCC_HDLC depends on QUICC_ENGINE, which in turn
> >> depends
> >>> on PPC32. As preparation for removing the latter and thus allowing the
> >>> core QE code to be built for other architectures, make FSL_UCC_HDLC
> >>> explicitly depend on PPC32.
> >>
> >> Is that really powerpc specific ? Can't the ARM QE perform HDLC on UCC ?
>
> I think the driver would build on ARM. Whether it works I don't know. I
> know it does not build on 64 bit hosts (see kbuild report for v2,23/23).

The problem for arm64 can be easy to fix.  Actually I don't think it
is neccessarily to be compiled on all architectures except powerpc,
arm and arm64.  I am surprised that you made the core QE code compile
for all architecture(although still have some kbuild warnings)

>
> > No.  Actually the HDLC and TDM are the major reason to integrate a QE on 
> > the ARM based Layerscape SoCs.
>
> [citation needed].

I got this message from our marketing team.  Also it is reflected on
marketing materials like
https://www.nxp.com/products/processors-and-microcontrollers/arm-processors/layerscape-communication-process/qoriq-layerscape-1043a-and-1023a-multicore-communications-processors:LS1043A

"The QorIQ LS1043A ... integrated QUICC Engine® for legacy glue-less
HDLC, TDM or Profibus support."

>
> > Since Rasmus doesn't have the hardware to test this feature Qiang Zhao 
> > probably can help verify the functionality of TDM and we can drop this 
> > patch.
>
> No, this patch cannot be dropped. Please see the kbuild complaints for
> v2,23/23 about use of IS_ERR_VALUE on not-sizeof(long) entities. I see
> kbuild has complained about the same thing for v3 since apparently the
> same thing appears in ucc_slow.c. So I'll fix that.

When I made this comment I didn't notice you have removed all the
architectural dependencies for CONFIG_QUICC_ENGINE.  If the
QUICC_ENGINE is only buidable on powerpc, arm and arm64, this change
will not be needed.

BTW, I'm not sure if it is a good idea to make it selectable on these
unrelavent architectures.  Real architectural dependencies and
COMPILE_TEST dependency will be better if we really want to test the
buildability on other platforms.

Regards,
Leo

Re: [PATCH -next] soc: fsl: Enable COMPILE_TEST

[Li Yang]

On Fri, Nov 8, 2019 at 9:20 AM Alexandre Belloni
 wrote:
>
> Hi,
>
> On 08/11/2019 21:02:13+0800, YueHaibing wrote:
> > When do COMPILE_TEST buiding for RTC_DRV_FSL_FTM_ALARM,
> > we get this warning:
> >
> > WARNING: unmet direct dependencies detected for FSL_RCPM
> >   Depends on [n]: PM_SLEEP [=y] && (ARM || ARM64)
> >   Selected by [m]:
> >   - RTC_DRV_FSL_FTM_ALARM [=m] && RTC_CLASS [=y] && (ARCH_LAYERSCAPE || 
> > SOC_LS1021A || COMPILE_TEST [=y])
> >
> > This enable COMPILE_TEST for FSL_RCPM to fix the issue.
> >
> > Fixes: e1c2feb1efa2 ("rtc: fsl-ftm-alarm: allow COMPILE_TEST")
>
> I've removed that patch until the fsl maintainers apply this one.

I think it is wrong to have RTC_DRV_FSL_FTM_ALARM select FSL_RCPM from
the begining.  The FTM_ALARM is primarily used as a wakeup source for
the deep sleep.  But it shouldn't be depending on it or selecting it.
I will create a patch to move that.

Regards,
Leo

>
> > Signed-off-by: YueHaibing 
> > ---
> > In commit c6c2d36bc46f ("rtc: fsl-ftm-alarm: Fix build error without 
> > PM_SLEEP")
> > I posted a wrong kconfig warning(which PM_SLEEP is n), sorry for confusion.
> > ---
> >  drivers/soc/fsl/Kconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig
> > index 4df32bc..e142662 100644
> > --- a/drivers/soc/fsl/Kconfig
> > +++ b/drivers/soc/fsl/Kconfig
> > @@ -43,7 +43,7 @@ config DPAA2_CONSOLE
> >
> >  config FSL_RCPM
> >   bool "Freescale RCPM support"
> > - depends on PM_SLEEP && (ARM || ARM64)
> > + depends on PM_SLEEP && (ARM || ARM64 || COMPILE_TEST)
> >   help
> > The NXP QorIQ Processors based on ARM Core have RCPM module
> > (Run Control and Power Management), which performs all device-level
> > --
> > 2.7.4
> >
> >
>
> --
> Alexandre Belloni, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

Re: [PATCH v3 35/36] net/wan: make FSL_UCC_HDLC explicitly depend on PPC32

[Li Yang]

On Tue, Nov 5, 2019 at 4:47 PM Rasmus Villemoes
 wrote:
>
> On 04/11/2019 21.56, Li Yang wrote:
>
> >> No, this patch cannot be dropped. Please see the kbuild complaints for
> >> v2,23/23 about use of IS_ERR_VALUE on not-sizeof(long) entities. I see
> >> kbuild has complained about the same thing for v3 since apparently the
> >> same thing appears in ucc_slow.c. So I'll fix that.
> >
> > When I made this comment I didn't notice you have removed all the
> > architectural dependencies for CONFIG_QUICC_ENGINE.  If the
> > QUICC_ENGINE is only buidable on powerpc, arm and arm64, this change
> > will not be needed.
> >
> > BTW, I'm not sure if it is a good idea to make it selectable on these
> > unrelavent architectures.  Real architectural dependencies and
> > COMPILE_TEST dependency will be better if we really want to test the
> > buildability on other platforms.
>
> Well, making QUICC_ENGINE depend on PPC32 || ARM would certainly make
> things easier for me. Once you include ARM64 or any other 64 bit
> architecture the buildbot complaints start rolling in from the
> IS_ERR_VALUEs. And ARM64 should be supported as well, so there really
> isn't much difference between dropping all arch restrictions and listing
> the relevant archs in the Kconfig dependencies.

I agree that it will be good to have the driver compile for all
architectures for compile test.  But list all the relevant
architectures and CONFIG_COMPILE_TEST as dependencies will make it not
really selected for these irrelevant architectures in real system.

Regards,
Leo

[PATCH] rtc: fsl-ftm-alarm: remove select FSL_RCPM and default y from Kconfig

[Li Yang]

The Flextimer alarm is primarily used as a wakeup source for system
power management.  But it shouldn't select the power management driver
as they don't really have dependency of each other.

Also remove the default y as it is not a critical feature for the
systems.

Signed-off-by: Li Yang 
---
 drivers/rtc/Kconfig | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 1adf9f815652..b33b80243143 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1337,8 +1337,6 @@ config RTC_DRV_IMXDI
 config RTC_DRV_FSL_FTM_ALARM
tristate "Freescale FlexTimer alarm timer"
depends on ARCH_LAYERSCAPE || SOC_LS1021A
-   select FSL_RCPM
-   default y
help
   For the FlexTimer in LS1012A, LS1021A, LS1028A, LS1043A, LS1046A,
   LS1088A, LS208xA, we can use FTM as the wakeup source.
-- 
2.17.1

Re: [PATCH v4 47/47] soc: fsl: qe: remove PPC32 dependency from CONFIG_QUICC_ENGINE

[Li Yang]

On Fri, Nov 8, 2019 at 7:05 AM Rasmus Villemoes
 wrote:
>
> There are also ARM and ARM64 based SOCs with a QUICC Engine, and the
> core QE code as well as net/wan/fsl_ucc_hdlc and tty/serial/ucc_uart
> has now been modified to not rely on ppcisms.
>
> So extend the architectures that can select QUICC_ENGINE, and add the
> rather modest requirements of OF && HAS_IOMEM.
>
> The core code as well as the ucc_uart driver has been tested on an
> LS1021A (arm), and it has also been tested that the QE code still
> works on an mpc8309 (ppc).
>
> Signed-off-by: Rasmus Villemoes 
> ---
>  drivers/soc/fsl/qe/Kconfig | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/fsl/qe/Kconfig b/drivers/soc/fsl/qe/Kconfig
> index cfa4b2939992..f1974f811572 100644
> --- a/drivers/soc/fsl/qe/Kconfig
> +++ b/drivers/soc/fsl/qe/Kconfig
> @@ -5,7 +5,8 @@
>
>  config QUICC_ENGINE
> bool "QUICC Engine (QE) framework support"
> -   depends on FSL_SOC && PPC32
> +   depends on OF && HAS_IOMEM
> +   depends on PPC32 || ARM || ARM64 || COMPILE_TEST

Can you also add PPC64?  It is also used on some PPC64 platforms
(QorIQ T series).

> select GENERIC_ALLOCATOR
> select CRC32
> help
> --
> 2.23.0
>

Re: [PATCH v4 47/47] soc: fsl: qe: remove PPC32 dependency from CONFIG_QUICC_ENGINE

[Li Yang]

On Mon, Nov 11, 2019 at 1:36 AM Rasmus Villemoes
 wrote:
>
> On 09/11/2019 00.48, Li Yang wrote:
> > On Fri, Nov 8, 2019 at 7:05 AM Rasmus Villemoes
> >  wrote:
> >>
> >> There are also ARM and ARM64 based SOCs with a QUICC Engine, and the
> >> core QE code as well as net/wan/fsl_ucc_hdlc and tty/serial/ucc_uart
> >> has now been modified to not rely on ppcisms.
> >>
> >> So extend the architectures that can select QUICC_ENGINE, and add the
> >> rather modest requirements of OF && HAS_IOMEM.
> >>
> >> The core code as well as the ucc_uart driver has been tested on an
> >> LS1021A (arm), and it has also been tested that the QE code still
> >> works on an mpc8309 (ppc).
> >>
> >> Signed-off-by: Rasmus Villemoes 
> >> ---
> >>  drivers/soc/fsl/qe/Kconfig | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/soc/fsl/qe/Kconfig b/drivers/soc/fsl/qe/Kconfig
> >> index cfa4b2939992..f1974f811572 100644
> >> --- a/drivers/soc/fsl/qe/Kconfig
> >> +++ b/drivers/soc/fsl/qe/Kconfig
> >> @@ -5,7 +5,8 @@
> >>
> >>  config QUICC_ENGINE
> >> bool "QUICC Engine (QE) framework support"
> >> -   depends on FSL_SOC && PPC32
> >> +   depends on OF && HAS_IOMEM
> >> +   depends on PPC32 || ARM || ARM64 || COMPILE_TEST
> >
> > Can you also add PPC64?  It is also used on some PPC64 platforms
> > (QorIQ T series).
>
> Sure, but if that's the only thing in the whole series, perhaps you
> could amend it when applying instead of me sending all 47 patches again.

Sure.  I can do that.

>
> Should PPC32 || PPC64 be spelled PPC?

Yes.  That will be good.

Regards,
Leo

Re: [PATCH v4 46/47] net: ethernet: freescale: make UCC_GETH explicitly depend on PPC32

[Li Yang]

On Thu, Nov 14, 2019 at 10:37 PM Timur Tabi  wrote:
>
> On Fri, Nov 8, 2019 at 7:04 AM Rasmus Villemoes
>  wrote:
> >
> > Currently, QUICC_ENGINE depends on PPC32, so this in itself does not
> > change anything. In order to allow removing the PPC32 dependency from
> > QUICC_ENGINE and avoid allmodconfig build failures, add this explicit
> > dependency.
>
> Can you add an explanation why we don't want ucc_geth on non-PowerPC 
> platforms?

I think it is because the QE Ethernet was never integrated in any
non-PowerPC SoC and most likely will not be in the future.  We
probably can make it compile for other architectures for general code
quality but it is not a priority.

Regards,
Leo

Re: [PATCH v4 00/47] QUICC Engine support on ARM and ARM64

[Li Yang]

On Mon, Nov 11, 2019 at 5:39 PM Li Yang  wrote:
>
> On Fri, Nov 8, 2019 at 7:05 AM Rasmus Villemoes
>  wrote:
> >
>
> I'm generally ok with these enhencements and cleanups.  But as the
> whole patch series touched multiple subsystems, I would like to
> collect the Acked-by from Scott, Greg and David if we want the whole
> series to go through the fsl/soc tree.

Rasmus,

Since the patches also touched net and serial subsystem.  Can you also
repost these patches(maybe just related ones) onto netdev and
linux-serial mailing list?

Regards,
Leo
>
> Also Qiang, can you help to test the latest version and provide you
> Tested-by?  Thanks.
>
> > There have been several attempts in the past few years to allow
> > building the QUICC engine drivers for platforms other than PPC. This
> > is yet another attempt.
> >
> > v3 can be found here: 
> > https://lore.kernel.org/lkml/20191101124210.14510-1-li...@rasmusvillemoes.dk/
> >
> > v4 adds a some patches to fix (ab)use of IS_ERR_VALUE which fails when
> > sizeof(u32) != sizeof(long), i.e. on 64-bit platforms. Freescale
> > drivers are some of the last holdouts using that macro (outside of
> > arch/ and core mm code), so I decided trying to simply get rid of it
> > instead of papering over it by using a temporary long to store the
> > result in. Doing that I stumbled on some other things that should be
> > fixed. These are the new patches 34-45.
> >
> > Patch 35 from v3 (which added a PPC32 dependency to FSL_UCC_HDLC) is
> > gone from this version, so that that driver can indeed now be built
> > for arm and arm64.
> >
> > 1-5 are about replacing in_be32 etc. in the core QE code 
> > (drivers/soc/fsl/qe).
> >
> > 6-8 handle miscellaneous other ppcisms.
> >
> > 9-21 deal with qe_ic: Simplifying the driver significantly by removing
> > unused code, and removing the platform-specific initialization from
> > arch/powerpc/.
> >
> > 22-25 deal with raw access to devicetree properties in native endianness.
> >
> > 26-33 makes drivers/tty/serial/ucc_uart.c (CONFIG_SERIAL_QE) ready to build 
> > on arm.
> >
> > 34-45 deal with IS_ERR_VALUE() and some other things found while
> > digging around that part of the code.
> >
> > 46 adds a PPC32 dependency to UCC_GETH - it has some of the same
> > issues that have been fixed in the ucc_uart and ucc_hdlc cases. Nobody
> > has requested that I allow that driver to be built for arm{,64}, so
> > instead of growing this series even bigger, I kept that addition. It's
> > trivial to remove if somebody cares enough to fix the build
> > errors/warnings and actually has a platform to test the result on.
> >
> > Finally patch 47 lifts the PPC32 restriction from QUICC_ENGINE. At the
> > request of Li Yang, it doesn't remove the PPC32 dependency but instead
> > changes it to PPC32 || ARM || ARM64 (or COMPILE_TEST), i.e. listing
> > the platforms that may have a QE.
> >
> > The series has been built and booted on both an mpc8309-based platform
> > (ppc) as well as an ls1021a-based platform (arm). The core QE code is
> > exercised on both, while I could only test the ucc_uart on arm, since
> > the uarts are not wired up on our mpc8309 board. Qiang Zhao reports
> > that the ucc_hdlc driver does indeed work on a ls1043ardb (arm64)
> > board, I hope he'll formally add a Tested-by: to the relevant patches
> > since I don't have any arm64 board with QE.
> >
> > Rasmus Villemoes (47):
> >   soc: fsl: qe: remove space-before-tab
> >   soc: fsl: qe: drop volatile qualifier of struct qe_ic::regs
> >   soc: fsl: qe: rename qe_(clr/set/clrset)bit* helpers
> >   soc: fsl: qe: introduce qe_io{read,write}* wrappers
> >   soc: fsl: qe: avoid ppc-specific io accessors
> >   soc: fsl: qe: replace spin_event_timeout by readx_poll_timeout_atomic
> >   soc: fsl: qe: qe.c: guard use of pvr_version_is() with CONFIG_PPC32
> >   soc: fsl: qe: drop unneeded #includes
> >   soc: fsl: qe: drop assign-only high_active in qe_ic_init
> >   soc: fsl: qe: remove pointless sysfs registration in qe_ic.c
> >   soc: fsl: qe: use qe_ic_cascade_{low,high}_mpic also on 83xx
> >   soc: fsl: qe: move calls of qe_ic_init out of arch/powerpc/
> >   powerpc/83xx: remove mpc83xx_ipic_and_qe_init_IRQ
> >   powerpc/85xx: remove mostly pointless mpc85xx_qe_init()
>
> Scott,
> What do you think about the PPC changes?
>
> >   soc: fsl: qe: move qe_ic_cascade_* functions to qe_ic.c
> >   soc: fsl: qe: rename qe_ic_cascade_low_mpic -> qe_ic_cascade_low
> >   soc: fsl: qe: remove unused qe_ic_set_* functions
&

Re: [PATCH v4 00/47] QUICC Engine support on ARM and ARM64

[Li Yang]

On Fri, Nov 8, 2019 at 7:05 AM Rasmus Villemoes
 wrote:
>

I'm generally ok with these enhencements and cleanups.  But as the
whole patch series touched multiple subsystems, I would like to
collect the Acked-by from Scott, Greg and David if we want the whole
series to go through the fsl/soc tree.

Also Qiang, can you help to test the latest version and provide you
Tested-by?  Thanks.

> There have been several attempts in the past few years to allow
> building the QUICC engine drivers for platforms other than PPC. This
> is yet another attempt.
>
> v3 can be found here: 
> https://lore.kernel.org/lkml/20191101124210.14510-1-li...@rasmusvillemoes.dk/
>
> v4 adds a some patches to fix (ab)use of IS_ERR_VALUE which fails when
> sizeof(u32) != sizeof(long), i.e. on 64-bit platforms. Freescale
> drivers are some of the last holdouts using that macro (outside of
> arch/ and core mm code), so I decided trying to simply get rid of it
> instead of papering over it by using a temporary long to store the
> result in. Doing that I stumbled on some other things that should be
> fixed. These are the new patches 34-45.
>
> Patch 35 from v3 (which added a PPC32 dependency to FSL_UCC_HDLC) is
> gone from this version, so that that driver can indeed now be built
> for arm and arm64.
>
> 1-5 are about replacing in_be32 etc. in the core QE code (drivers/soc/fsl/qe).
>
> 6-8 handle miscellaneous other ppcisms.
>
> 9-21 deal with qe_ic: Simplifying the driver significantly by removing
> unused code, and removing the platform-specific initialization from
> arch/powerpc/.
>
> 22-25 deal with raw access to devicetree properties in native endianness.
>
> 26-33 makes drivers/tty/serial/ucc_uart.c (CONFIG_SERIAL_QE) ready to build 
> on arm.
>
> 34-45 deal with IS_ERR_VALUE() and some other things found while
> digging around that part of the code.
>
> 46 adds a PPC32 dependency to UCC_GETH - it has some of the same
> issues that have been fixed in the ucc_uart and ucc_hdlc cases. Nobody
> has requested that I allow that driver to be built for arm{,64}, so
> instead of growing this series even bigger, I kept that addition. It's
> trivial to remove if somebody cares enough to fix the build
> errors/warnings and actually has a platform to test the result on.
>
> Finally patch 47 lifts the PPC32 restriction from QUICC_ENGINE. At the
> request of Li Yang, it doesn't remove the PPC32 dependency but instead
> changes it to PPC32 || ARM || ARM64 (or COMPILE_TEST), i.e. listing
> the platforms that may have a QE.
>
> The series has been built and booted on both an mpc8309-based platform
> (ppc) as well as an ls1021a-based platform (arm). The core QE code is
> exercised on both, while I could only test the ucc_uart on arm, since
> the uarts are not wired up on our mpc8309 board. Qiang Zhao reports
> that the ucc_hdlc driver does indeed work on a ls1043ardb (arm64)
> board, I hope he'll formally add a Tested-by: to the relevant patches
> since I don't have any arm64 board with QE.
>
> Rasmus Villemoes (47):
>   soc: fsl: qe: remove space-before-tab
>   soc: fsl: qe: drop volatile qualifier of struct qe_ic::regs
>   soc: fsl: qe: rename qe_(clr/set/clrset)bit* helpers
>   soc: fsl: qe: introduce qe_io{read,write}* wrappers
>   soc: fsl: qe: avoid ppc-specific io accessors
>   soc: fsl: qe: replace spin_event_timeout by readx_poll_timeout_atomic
>   soc: fsl: qe: qe.c: guard use of pvr_version_is() with CONFIG_PPC32
>   soc: fsl: qe: drop unneeded #includes
>   soc: fsl: qe: drop assign-only high_active in qe_ic_init
>   soc: fsl: qe: remove pointless sysfs registration in qe_ic.c
>   soc: fsl: qe: use qe_ic_cascade_{low,high}_mpic also on 83xx
>   soc: fsl: qe: move calls of qe_ic_init out of arch/powerpc/
>   powerpc/83xx: remove mpc83xx_ipic_and_qe_init_IRQ
>   powerpc/85xx: remove mostly pointless mpc85xx_qe_init()

Scott,
What do you think about the PPC changes?

>   soc: fsl: qe: move qe_ic_cascade_* functions to qe_ic.c
>   soc: fsl: qe: rename qe_ic_cascade_low_mpic -> qe_ic_cascade_low
>   soc: fsl: qe: remove unused qe_ic_set_* functions
>   soc: fsl: qe: don't use NO_IRQ in qe_ic.c
>   soc: fsl: qe: make qe_ic_get_{low,high}_irq static
>   soc: fsl: qe: simplify qe_ic_init()
>   soc: fsl: qe: merge qe_ic.h headers into qe_ic.c
>   soc: fsl: qe: qe.c: use of_property_read_* helpers
>   soc: fsl: qe: qe_io.c: don't open-code of_parse_phandle()
>   soc: fsl: qe: qe_io.c: access device tree property using be32_to_cpu
>   soc: fsl: qe: qe_io.c: use of_property_read_u32() in par_io_init()
>   soc: fsl: move cpm.h from powerpc/include/asm to include/soc/fsl
>   soc/fsl/qe/qe.h: update include path for cpm.h
>   serial: ucc_uart: explicitly include soc/fsl/cpm.h
>   serial: ucc_uart:

Re: [PATCH v6 00/49] QUICC Engine support on ARM, ARM64, PPC64

[Li Yang]

On Thu, Nov 28, 2019 at 8:59 AM Rasmus Villemoes
 wrote:
>
> There have been several attempts in the past few years to allow
> building the QUICC engine drivers for platforms other than PPC32. This
> is yet another attempt.
>
> v5 can be found here: 
> https://lore.kernel.org/lkml/20191118112324.22725-1-li...@rasmusvillemoes.dk/
>
> Changes in v6:
>
> - add various R-b, A-b tags
>
> - add a patch (48/49) fixing a build issue on ARM with CONFIG_SMP=y
>
> I added that patch last in the series, apart from the "allow to build
> on ARM" Kconfig change, to preserve the enumeration of the other
> patches from v5.
>
> 1-5 are about replacing in_be32 etc. in the core QE code (drivers/soc/fsl/qe).
>
> 6-8 handle miscellaneous other ppcisms.
>
> 9-21 deal with qe_ic: Simplifying the driver significantly by removing
> unused code, and removing the platform-specific initialization from
> arch/powerpc/.
>
> 22-25 deal with raw access to devicetree properties in native endianness.
>
> 26-34 makes drivers/tty/serial/ucc_uart.c (CONFIG_SERIAL_QE) ready to build 
> on non-ppc.
>
> 35-46 deal with IS_ERR_VALUE() and some other things found while
> digging around that part of the code.
>
> 47 adds a PPC32 dependency to UCC_GETH - it has some of the same
> issues that have been fixed in the ucc_uart and ucc_hdlc cases. Nobody
> has requested that I allow that driver to be built for arm{,64} and
> reportedly, the hardware has only ever shipped on PPC SOCs. So instead
> of growing this series even bigger, I kept that addition. It's trivial
> to remove if somebody cares enough to fix the build errors/warnings
> and actually has a platform to test the result on.
>
> 48 fixes a build issue on ARM reported by the kbuild bot.
>
> Finally patch 49 lifts the PPC32 restriction from QUICC_ENGINE. At the
> request of Li Yang, it doesn't remove the PPC32 dependency but instead
> changes it to PPC|| ARM || ARM64 (or COMPILE_TEST), i.e. listing
> the platforms that may have a QE.
>
> The series has been built and booted on both an mpc8309-based platform
> (ppc) as well as an ls1021a-based platform (arm). The core QE code is
> exercised on both, while I could only test the ucc_uart on arm, since
> the uarts are not wired up on our mpc8309 board. Qiang Zhao reports
> that the ucc_hdlc driver does indeed work on a ls1043ardb (arm64)
> board.

Series applied for next on my soc tree.  Thanks!

Regards,
Leo
>
> Rasmus Villemoes (49):
>   soc: fsl: qe: remove space-before-tab
>   soc: fsl: qe: drop volatile qualifier of struct qe_ic::regs
>   soc: fsl: qe: rename qe_(clr/set/clrset)bit* helpers
>   soc: fsl: qe: introduce qe_io{read,write}* wrappers
>   soc: fsl: qe: avoid ppc-specific io accessors
>   soc: fsl: qe: replace spin_event_timeout by readx_poll_timeout_atomic
>   soc: fsl: qe: qe.c: guard use of pvr_version_is() with CONFIG_PPC32
>   soc: fsl: qe: drop unneeded #includes
>   soc: fsl: qe: drop assign-only high_active in qe_ic_init
>   soc: fsl: qe: remove pointless sysfs registration in qe_ic.c
>   soc: fsl: qe: use qe_ic_cascade_{low,high}_mpic also on 83xx
>   soc: fsl: qe: move calls of qe_ic_init out of arch/powerpc/
>   powerpc/83xx: remove mpc83xx_ipic_and_qe_init_IRQ
>   powerpc/85xx: remove mostly pointless mpc85xx_qe_init()
>   soc: fsl: qe: move qe_ic_cascade_* functions to qe_ic.c
>   soc: fsl: qe: rename qe_ic_cascade_low_mpic -> qe_ic_cascade_low
>   soc: fsl: qe: remove unused qe_ic_set_* functions
>   soc: fsl: qe: don't use NO_IRQ in qe_ic.c
>   soc: fsl: qe: make qe_ic_get_{low,high}_irq static
>   soc: fsl: qe: simplify qe_ic_init()
>   soc: fsl: qe: merge qe_ic.h headers into qe_ic.c
>   soc: fsl: qe: qe.c: use of_property_read_* helpers
>   soc: fsl: qe: qe_io.c: don't open-code of_parse_phandle()
>   soc: fsl: qe: qe_io.c: access device tree property using be32_to_cpu
>   soc: fsl: qe: qe_io.c: use of_property_read_u32() in par_io_init()
>   soc: fsl: move cpm.h from powerpc/include/asm to include/soc/fsl
>   soc/fsl/qe/qe.h: update include path for cpm.h
>   serial: ucc_uart: explicitly include soc/fsl/cpm.h
>   serial: ucc_uart: replace ppc-specific IO accessors
>   serial: ucc_uart: factor out soft_uart initialization
>   serial: ucc_uart: stub out soft_uart_init for !CONFIG_PPC32
>   serial: ucc_uart: use of_property_read_u32() in ucc_uart_probe()
>   serial: ucc_uart: limit brg-frequency workaround to PPC32
>   serial: ucc_uart: access __be32 field using be32_to_cpu
>   soc: fsl: qe: change return type of cpm_muram_alloc() to s32
>   soc: fsl: qe: make cpm_muram_free() return void
>   soc: fsl: qe: make cpm_muram_free() ignore a negative offset
>   soc: fsl: qe: drop broken lazy call of cpm_muram_init()
>   soc: fs

Re: [PATCH 0/7] towards QE support on ARM

[Li Yang]

On Mon, Oct 21, 2019 at 3:46 AM Rasmus Villemoes
 wrote:
>
> On 18/10/2019 23.52, Li Yang wrote:
> > On Fri, Oct 18, 2019 at 3:54 PM Rasmus Villemoes
> >  wrote:
> >>
> >> On 18/10/2019 22.16, Leo Li wrote:
> >>>
> >>>>
> >>>> There have been several attempts in the past few years to allow building 
> >>>> the
> >>>> QUICC engine drivers for platforms other than PPC. This is (the 
> >>>> beginning of)
> >>>> yet another attempt. I hope I can get someone to pick up these relatively
> >>>> trivial patches (I _think_ they shouldn't change functionality at all), 
> >>>> and then
> >>>> I'll continue slowly working towards removing the PPC32 dependency for
> >>>> CONFIG_QUICC_ENGINE.
> >>>
> >>> Hi Rasmus,
> >>>
> >>> I don't fully understand the motivation of this work.  As far as I know 
> >>> the QUICC ENGINE is only used on PowerPC based SoCs.
> >>
> >> Hm, you're not the Leo Li that participated in this thread
> >> <https://lore.kernel.org/lkml/am3pr04mb11857ae8d2b0be56121b97d391...@am3pr04mb1185.eurprd04.prod.outlook.com/T/#u>?
> >
> > Oops, I totally forgot about this discussion which is just three years
> > ago.  :)  The QE-HDLC on LS1021a is kind of a special case.
> >
> >>
> >>
> >>  Can you give an example on how is it used on ARM system?
> >>
> >> LS1021A, for example, which is the one I'm aiming for getting fully
> >> supported in mainline.
> >> <https://www.nxp.com/products/processors-and-microcontrollers/arm-processors/layerscape-communication-process/qoriq-layerscape-1021a-dual-core-communications-processor-with-lcd-controller:LS1021A>
> >>
> >> The forks at https://github.com/qoriq-open-source/linux.git have various
> >> degrees of support (grep for commits saying stuff like "remove PPCisms"
> >> - some versions can be found on
> >> <https://lore.kernel.org/lkml/?q=remove+ppcisms>). Our current kernel is
> >> based on commits from the now-vanished 4.1 branch, and unfortunately at
> >> least the 4.14 branch (LSDK-18.06-V4.14) trivially doesn't build on ARM,
> >> despite the PPC32 dependency having been removed from CONFIG_QUICC_ENGINE.
> >
> > Can you try the 4.14 branch from a newer LSDK release?  LS1021a should
> > be supported platform on LSDK.  If it is broken, something is wrong.
>
> What newer release? LSDK-18.06-V4.14 is the latest -V4.14 tag at
> https://github.com/qoriq-open-source/linux.git, and identical to the

That tree has been abandoned for a while, we probably should state
that in the github.  The latest tree can be found at
https://source.codeaurora.org/external/qoriq/qoriq-components/linux/

> linux-4.14 branch. And despite commit 4c33e2d0576b removing the PPC32
> dependency from QUICC_ENGINE, it clearly hasn't been built on arm, since
> back around v4.12, mainline's qe.c grew a call to pvr_version_is which
> is ppc-only. So from that I sort of assumed that NXP had dropped trying
> to support the LS1021A even in their own kernels.
>
> In any case, we have zero interest in running an NXP kernel. Maybe I
> should clarify what I meant by "based on commits from" above: We're
> currently running a mainline 4.14 kernel on LS1021A, with a few patches
> inspired from the NXP 4.1 branch applied on top - but also with some
> manual fixes for e.g. the pvr_version_is() issue. Now we want to move
> that to a 4.19-based kernel (so that it aligns with our MPC8309 platform).

We also provide 4.19 based kernel in the codeaurora repo.  I think it
will be helpful to reuse patches there if you want to make your own
tree.

>
> >> This is just some first few steps, and I'm not claiming
> >> that this is sufficient to make the QE drivers build on ARM yet. But I
> >> have a customer with both mpc8309-based and ls1021a-based platforms, and
> >> they want to run the same, as-close-to-mainline-as-possible, kernel on
> >> both. So I will take a piecemeal approach, and try to make sure I don't
> >> break the ppc boards in the process (just building and booting one board
> >> is of course not sufficient, but better than nothing). Once I get to
> >> actually build some of the QE drivers for ARM, I'll of course also test
> >> them.
> >
> > Understood.  Zhao Qiang also maintains some patches similar to your
> > patchset and I think they are tested on ARM.  But the review of these
> > patches from last submission didn't finish.  It looks like your
> > patches are 

Re: [PATCH 0/7] towards QE support on ARM

[Li Yang]

On Tue, Oct 22, 2019 at 9:54 PM Qiang Zhao  wrote:
>
> On 22/10/2019 18:18, Rasmus Villemoes  wrote:
> > -Original Message-
> > From: Rasmus Villemoes 
> > Sent: 2019年10月22日 18:18
> > To: Qiang Zhao ; Leo Li 
> > Cc: Timur Tabi ; Greg Kroah-Hartman
> > ; linux-ker...@vger.kernel.org;
> > linux-ser...@vger.kernel.org; Jiri Slaby ;
> > linuxppc-dev@lists.ozlabs.org; linux-arm-ker...@lists.infradead.org
> > Subject: Re: [PATCH 0/7] towards QE support on ARM
> >
> > On 22/10/2019 04.24, Qiang Zhao wrote:
> > > On Mon, Oct 22, 2019 at 6:11 AM Leo Li wrote
> >
> > >> Right.  I'm really interested in getting this applied to my tree and
> > >> make it upstream.  Zhao Qiang, can you help to review Rasmus's
> > >> patches and comment?
> > >
> > > As you know, I maintained a similar patchset removing PPC, and someone
> > told me qe_ic should moved into drivers/irqchip/.
> > > I also thought qe_ic is a interrupt control driver, should be moved into 
> > > dir
> > irqchip.
> >
> > Yes, and I also plan to do that at some point. However, that's orthogonal to
> > making the driver build on ARM, so I don't want to mix the two. Making it
> > usable on ARM is my/our priority currently.
> >
> > I'd appreciate your input on my patches.
>
> Yes, we can put this patchset in first place, ensure it can build and work on 
> ARM, then push another patchset to move qe_ic.

Right.  I would only accept a patch series that can really build and
work on ARM.  At least the current out-of-tree patches can make it
work on ARM.  If we accept partial changes, there is no way to make it
work on the latest kernel on ARM then.

Regards,
Leo

Re: [PATCH v10 1/3] PM: wakeup: Add routine to help fetch wakeup source object.

[Li Yang]

On Thu, Oct 24, 2019 at 4:29 AM Ran Wang  wrote:
>
> Some user might want to go through all registered wakeup sources
> and doing things accordingly. For example, SoC PM driver might need to
> do HW programming to prevent powering down specific IP which wakeup
> source depending on. So add this API to help walk through all registered
> wakeup source objects on that list and return them one by one.
>
> Signed-off-by: Ran Wang 
> Tested-by: Leonard Crestez 
> Reviewed-by: Rafael J. Wysocki 

Series applied to soc/fsl for next.  Thanks.

Regards,
Leo
> ---
> Change in v10:
> - Add 'Reviewed-by: Rafael J. Wysocki '
>   to commit message.
>
> Change in v9:
> - Supplement comments for wakeup_sources_read_lock(),
>   wakeup_sources_read_unlock, wakeup_sources_walk_start and
>   wakeup_sources_walk_next().
>
> Change in v8:
> - Rename wakeup_source_get_next() to wakeup_sources_walk_next().
> - Add wakeup_sources_read_lock() to take over locking job of
>   wakeup_source_get_star().
> - Rename wakeup_source_get_start() to wakeup_sources_walk_start().
> - Replace wakeup_source_get_stop() with wakeup_sources_read_unlock().
> - Define macro for_each_wakeup_source(ws).
>
> Change in v7:
> - Remove define of member *dev in wake_irq to fix conflict with commit
> c8377adfa781 ("PM / wakeup: Show wakeup sources stats in sysfs"), user
> will use ws->dev->parent instead.
> - Remove '#include ' because it is not used.
>
> Change in v6:
> - Add wakeup_source_get_star() and wakeup_source_get_stop() to aligned
> with wakeup_sources_stats_seq_start/nex/stop.
>
> Change in v5:
> - Update commit message, add decription of walk through all wakeup
> source objects.
> - Add SCU protection in function wakeup_source_get_next().
> - Rename wakeup_source member 'attached_dev' to 'dev' and move it up
> (before wakeirq).
>
> Change in v4:
> - None.
>
> Change in v3:
> - Adjust indentation of *attached_dev;.
>
> Change in v2:
> - None.
>
>  drivers/base/power/wakeup.c | 54 
> +
>  include/linux/pm_wakeup.h   |  9 
>  2 files changed, 63 insertions(+)
>
> diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
> index 5817b51..70a9edb 100644
> --- a/drivers/base/power/wakeup.c
> +++ b/drivers/base/power/wakeup.c
> @@ -248,6 +248,60 @@ void wakeup_source_unregister(struct wakeup_source *ws)
>  EXPORT_SYMBOL_GPL(wakeup_source_unregister);
>
>  /**
> + * wakeup_sources_read_lock - Lock wakeup source list for read.
> + *
> + * Returns an index of srcu lock for struct wakeup_srcu.
> + * This index must be passed to the matching wakeup_sources_read_unlock().
> + */
> +int wakeup_sources_read_lock(void)
> +{
> +   return srcu_read_lock(_srcu);
> +}
> +EXPORT_SYMBOL_GPL(wakeup_sources_read_lock);
> +
> +/**
> + * wakeup_sources_read_unlock - Unlock wakeup source list.
> + * @idx: return value from corresponding wakeup_sources_read_lock()
> + */
> +void wakeup_sources_read_unlock(int idx)
> +{
> +   srcu_read_unlock(_srcu, idx);
> +}
> +EXPORT_SYMBOL_GPL(wakeup_sources_read_unlock);
> +
> +/**
> + * wakeup_sources_walk_start - Begin a walk on wakeup source list
> + *
> + * Returns first object of the list of wakeup sources.
> + *
> + * Note that to be safe, wakeup sources list needs to be locked by calling
> + * wakeup_source_read_lock() for this.
> + */
> +struct wakeup_source *wakeup_sources_walk_start(void)
> +{
> +   struct list_head *ws_head = _sources;
> +
> +   return list_entry_rcu(ws_head->next, struct wakeup_source, entry);
> +}
> +EXPORT_SYMBOL_GPL(wakeup_sources_walk_start);
> +
> +/**
> + * wakeup_sources_walk_next - Get next wakeup source from the list
> + * @ws: Previous wakeup source object
> + *
> + * Note that to be safe, wakeup sources list needs to be locked by calling
> + * wakeup_source_read_lock() for this.
> + */
> +struct wakeup_source *wakeup_sources_walk_next(struct wakeup_source *ws)
> +{
> +   struct list_head *ws_head = _sources;
> +
> +   return list_next_or_null_rcu(ws_head, >entry,
> +   struct wakeup_source, entry);
> +}
> +EXPORT_SYMBOL_GPL(wakeup_sources_walk_next);
> +
> +/**
>   * device_wakeup_attach - Attach a wakeup source object to a device object.
>   * @dev: Device to handle.
>   * @ws: Wakeup source object to attach to @dev.
> diff --git a/include/linux/pm_wakeup.h b/include/linux/pm_wakeup.h
> index 661efa0..aa3da66 100644
> --- a/include/linux/pm_wakeup.h
> +++ b/include/linux/pm_wakeup.h
> @@ -63,6 +63,11 @@ struct wakeup_source {
> boolautosleep_enabled:1;
>  };
>
> +#define for_each_wakeup_source(ws) \
> +   for ((ws) = wakeup_sources_walk_start();\
> +(ws);  \
> +(ws) = 

Re: [PATCH v9 3/3] soc: fsl: add RCPM driver

[Li Yang]

On Wed, Oct 23, 2019 at 3:24 AM Ran Wang  wrote:
>
> The NXP's QorIQ Processors based on ARM Core have RCPM module

Actually not just ARM based QorIQ processors are having RCPM, PowerPC
based QorIQ SoCs also have RCPM.  Does this driver also work with the
PowerPC SoCs?  Please clarify in the commit message and Kconfig
description.

> (Run Control and Power Management), which performs system level
> tasks associated with power management such as wakeup source control.
>
> This driver depends on PM wakeup source framework which help to
> collect wake information.
>
> Signed-off-by: Ran Wang 
> ---
> Change in v9:
> - Add kerneldoc for rcpm_pm_prepare().
> - Use pr_debug() to replace dev_info(), to print message when decide
>   skip current wakeup object, this is mainly for debugging (in order
>   to detect potential improper implementation on device tree which
>   might cause this skip).
> - Refactor looping implementation in rcpm_pm_prepare(), add more
>   comments to help clarify.
>
> Change in v8:
> - Adjust related API usage to meet wakeup.c's update in patch 1/3.
> - Add sanity checking for the case of ws->dev or ws->dev->parent
>   is null.
>
> Change in v7:
> - Replace 'ws->dev' with 'ws->dev->parent' to get aligned with
> c8377adfa781 ("PM / wakeup: Show wakeup sources stats in sysfs")
> - Remove '+obj-y += ftm_alarm.o' since it is wrong.
> - Cosmetic work.
>
> Change in v6:
> - Adjust related API usage to meet wakeup.c's update in patch 1/3.
>
> Change in v5:
> - Fix v4 regression of the return value of wakeup_source_get_next()
> didn't pass to ws in while loop.
> - Rename wakeup_source member 'attached_dev' to 'dev'.
> - Rename property 'fsl,#rcpm-wakeup-cells' to 
> '#fsl,rcpm-wakeup-cells'.
> please see https://lore.kernel.org/patchwork/patch/1101022/
>
> Change in v4:
> - Remove extra ',' in author line of rcpm.c
> - Update usage of wakeup_source_get_next() to be less confusing to the
> reader, code logic remain the same.
>
> Change in v3:
> - Some whitespace ajdustment.
>
> Change in v2:
> - Rebase Kconfig and Makefile update to latest mainline.
>
>  drivers/soc/fsl/Kconfig  |   8 +++
>  drivers/soc/fsl/Makefile |   1 +
>  drivers/soc/fsl/rcpm.c   | 148 
> +++
>  3 files changed, 157 insertions(+)
>  create mode 100644 drivers/soc/fsl/rcpm.c
>
> diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig
> index f9ad8ad..4918856 100644
> --- a/drivers/soc/fsl/Kconfig
> +++ b/drivers/soc/fsl/Kconfig
> @@ -40,4 +40,12 @@ config DPAA2_CONSOLE
>   /dev/dpaa2_mc_console and /dev/dpaa2_aiop_console,
>   which can be used to dump the Management Complex and AIOP
>   firmware logs.
> +
> +config FSL_RCPM
> +   bool "Freescale RCPM support"
> +   depends on PM_SLEEP

If this is only for ARM, probably add more dependency here?

> +   help
> + The NXP QorIQ Processors based on ARM Core have RCPM module
> + (Run Control and Power Management), which performs all device-level
> + tasks associated with power management, such as wakeup source 
> control.
>  endmenu
> diff --git a/drivers/soc/fsl/Makefile b/drivers/soc/fsl/Makefile
> index 71dee8d..906f1cd 100644
> --- a/drivers/soc/fsl/Makefile
> +++ b/drivers/soc/fsl/Makefile
> @@ -6,6 +6,7 @@
>  obj-$(CONFIG_FSL_DPAA) += qbman/
>  obj-$(CONFIG_QUICC_ENGINE) += qe/
>  obj-$(CONFIG_CPM)  += qe/
> +obj-$(CONFIG_FSL_RCPM) += rcpm.o
>  obj-$(CONFIG_FSL_GUTS) += guts.o
>  obj-$(CONFIG_FSL_MC_DPIO)  += dpio/
>  obj-$(CONFIG_DPAA2_CONSOLE)+= dpaa2-console.o
> diff --git a/drivers/soc/fsl/rcpm.c b/drivers/soc/fsl/rcpm.c
> new file mode 100644
> index 000..9378073
> --- /dev/null
> +++ b/drivers/soc/fsl/rcpm.c
> @@ -0,0 +1,148 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// rcpm.c - Freescale QorIQ RCPM driver
> +//
> +// Copyright 2019 NXP
> +//
> +// Author: Ran Wang 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define RCPM_WAKEUP_CELL_MAX_SIZE  7
> +
> +struct rcpm {
> +   unsigned intwakeup_cells;
> +   void __iomem*ippdexpcr_base;
> +   boollittle_endian;
> +};
> +
> +/**
> + * rcpm_pm_prepare - performs device-level tasks associated with power
> + * management, such as programming related to the wakeup source control.
> + * @dev: Device to handle.
> + *
> + */
> +static int rcpm_pm_prepare(struct device *dev)
> +{
> +   int i, ret, idx;
> +   void __iomem *base;
> +   struct wakeup_source*ws;
> +   struct rcpm *rcpm;
> +   struct device_node  *np = dev->of_node;
> +   u32 value[RCPM_WAKEUP_CELL_MAX_SIZE + 1];
> +
> +  

Re: [PATCH 0/7] towards QE support on ARM

[Li Yang]

On Fri, Oct 18, 2019 at 3:54 PM Rasmus Villemoes
 wrote:
>
> On 18/10/2019 22.16, Leo Li wrote:
> >
> >>
> >> There have been several attempts in the past few years to allow building 
> >> the
> >> QUICC engine drivers for platforms other than PPC. This is (the beginning 
> >> of)
> >> yet another attempt. I hope I can get someone to pick up these relatively
> >> trivial patches (I _think_ they shouldn't change functionality at all), 
> >> and then
> >> I'll continue slowly working towards removing the PPC32 dependency for
> >> CONFIG_QUICC_ENGINE.
> >
> > Hi Rasmus,
> >
> > I don't fully understand the motivation of this work.  As far as I know the 
> > QUICC ENGINE is only used on PowerPC based SoCs.
>
> Hm, you're not the Leo Li that participated in this thread
> ?

Oops, I totally forgot about this discussion which is just three years
ago.  :)  The QE-HDLC on LS1021a is kind of a special case.

>
>
>  Can you give an example on how is it used on ARM system?
>
> LS1021A, for example, which is the one I'm aiming for getting fully
> supported in mainline.
> 
>
> The forks at https://github.com/qoriq-open-source/linux.git have various
> degrees of support (grep for commits saying stuff like "remove PPCisms"
> - some versions can be found on
> ). Our current kernel is
> based on commits from the now-vanished 4.1 branch, and unfortunately at
> least the 4.14 branch (LSDK-18.06-V4.14) trivially doesn't build on ARM,
> despite the PPC32 dependency having been removed from CONFIG_QUICC_ENGINE.

Can you try the 4.14 branch from a newer LSDK release?  LS1021a should
be supported platform on LSDK.  If it is broken, something is wrong.

>
> >>
> >> Tested on an MPC8309-derived board.
> >
> > MPC8309 is also PPC based.
>
> True, of course. This is just some first few steps, and I'm not claiming
> that this is sufficient to make the QE drivers build on ARM yet. But I
> have a customer with both mpc8309-based and ls1021a-based platforms, and
> they want to run the same, as-close-to-mainline-as-possible, kernel on
> both. So I will take a piecemeal approach, and try to make sure I don't
> break the ppc boards in the process (just building and booting one board
> is of course not sufficient, but better than nothing). Once I get to
> actually build some of the QE drivers for ARM, I'll of course also test
> them.

Understood.  Zhao Qiang also maintains some patches similar to your
patchset and I think they are tested on ARM.  But the review of these
patches from last submission didn't finish.  It looks like your
patches are better divided but not really verified on ARM.  Zhao
Qiang's patches are tested but maybe need some final touch for
cleaning up.  I will let you guys decide what is the best approach to
make this upstreamed.

Regards,
Leo

Re: [PATCH v5 45/48] net/wan/fsl_ucc_hdlc: fix reading of __be16 registers

[Li Yang]

On Mon, Nov 18, 2019 at 5:26 AM Rasmus Villemoes
 wrote:
>

Hi David,

What do you think about the patch 45-47 from the series for net
related changes?  If it is ok with you, I can merge them with the
whole series through the soc tree with your ACK.

Regards,
Leo

> When releasing the allocated muram resource, we rely on reading back
> the offsets from the riptr/tiptr registers. But those registers are
> __be16 (and we indeed write them using iowrite16be), so we can't just
> read them back with a normal C dereference.
>
> This is not currently a real problem, since for now the driver is
> PPC32-only. But it will soon be allowed to be used on arm and arm64 as
> well.
>
> Signed-off-by: Rasmus Villemoes 
> ---
>  drivers/net/wan/fsl_ucc_hdlc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
> index 405b24a5a60d..8d13586bb774 100644
> --- a/drivers/net/wan/fsl_ucc_hdlc.c
> +++ b/drivers/net/wan/fsl_ucc_hdlc.c
> @@ -732,8 +732,8 @@ static int uhdlc_open(struct net_device *dev)
>
>  static void uhdlc_memclean(struct ucc_hdlc_private *priv)
>  {
> -   qe_muram_free(priv->ucc_pram->riptr);
> -   qe_muram_free(priv->ucc_pram->tiptr);
> +   qe_muram_free(ioread16be(>ucc_pram->riptr));
> +   qe_muram_free(ioread16be(>ucc_pram->tiptr));
>
> if (priv->rx_bd_base) {
> dma_free_coherent(priv->dev,
> --
> 2.23.0
>

Re: [PATCH v5 13/48] powerpc/83xx: remove mpc83xx_ipic_and_qe_init_IRQ

[Li Yang]

On Mon, Nov 18, 2019 at 5:29 AM Rasmus Villemoes
 wrote:

Hi Scott,

What do you think of the PowerPC related changes(patch 13,14)?  Can we
have you ACK and merge the series from soc tree?

Regards,
Leo
>
> This is now exactly the same as mpc83xx_ipic_init_IRQ, so just use
> that directly.
>
> Signed-off-by: Rasmus Villemoes 
> ---
>  arch/powerpc/platforms/83xx/km83xx.c  | 2 +-
>  arch/powerpc/platforms/83xx/misc.c| 7 ---
>  arch/powerpc/platforms/83xx/mpc832x_mds.c | 2 +-
>  arch/powerpc/platforms/83xx/mpc832x_rdb.c | 2 +-
>  arch/powerpc/platforms/83xx/mpc836x_mds.c | 2 +-
>  arch/powerpc/platforms/83xx/mpc836x_rdk.c | 2 +-
>  arch/powerpc/platforms/83xx/mpc83xx.h | 5 -
>  7 files changed, 5 insertions(+), 17 deletions(-)
>
> diff --git a/arch/powerpc/platforms/83xx/km83xx.c 
> b/arch/powerpc/platforms/83xx/km83xx.c
> index 5c6227f7bc37..3d89569e9e71 100644
> --- a/arch/powerpc/platforms/83xx/km83xx.c
> +++ b/arch/powerpc/platforms/83xx/km83xx.c
> @@ -177,7 +177,7 @@ define_machine(mpc83xx_km) {
> .name   = "mpc83xx-km-platform",
> .probe  = mpc83xx_km_probe,
> .setup_arch = mpc83xx_km_setup_arch,
> -   .init_IRQ   = mpc83xx_ipic_and_qe_init_IRQ,
> +   .init_IRQ   = mpc83xx_ipic_init_IRQ,
> .get_irq= ipic_get_irq,
> .restart= mpc83xx_restart,
> .time_init  = mpc83xx_time_init,
> diff --git a/arch/powerpc/platforms/83xx/misc.c 
> b/arch/powerpc/platforms/83xx/misc.c
> index 6935a5b9fbd1..1d8306eb2958 100644
> --- a/arch/powerpc/platforms/83xx/misc.c
> +++ b/arch/powerpc/platforms/83xx/misc.c
> @@ -88,13 +88,6 @@ void __init mpc83xx_ipic_init_IRQ(void)
> ipic_set_default_priority();
>  }
>
> -#ifdef CONFIG_QUICC_ENGINE
> -void __init mpc83xx_ipic_and_qe_init_IRQ(void)
> -{
> -   mpc83xx_ipic_init_IRQ();
> -}
> -#endif /* CONFIG_QUICC_ENGINE */
> -
>  static const struct of_device_id of_bus_ids[] __initconst = {
> { .type = "soc", },
> { .compatible = "soc", },
> diff --git a/arch/powerpc/platforms/83xx/mpc832x_mds.c 
> b/arch/powerpc/platforms/83xx/mpc832x_mds.c
> index 1c73af104d19..6fa5402ebf20 100644
> --- a/arch/powerpc/platforms/83xx/mpc832x_mds.c
> +++ b/arch/powerpc/platforms/83xx/mpc832x_mds.c
> @@ -101,7 +101,7 @@ define_machine(mpc832x_mds) {
> .name   = "MPC832x MDS",
> .probe  = mpc832x_sys_probe,
> .setup_arch = mpc832x_sys_setup_arch,
> -   .init_IRQ   = mpc83xx_ipic_and_qe_init_IRQ,
> +   .init_IRQ   = mpc83xx_ipic_init_IRQ,
> .get_irq= ipic_get_irq,
> .restart= mpc83xx_restart,
> .time_init  = mpc83xx_time_init,
> diff --git a/arch/powerpc/platforms/83xx/mpc832x_rdb.c 
> b/arch/powerpc/platforms/83xx/mpc832x_rdb.c
> index 87f68ca06255..622c625d5ce4 100644
> --- a/arch/powerpc/platforms/83xx/mpc832x_rdb.c
> +++ b/arch/powerpc/platforms/83xx/mpc832x_rdb.c
> @@ -219,7 +219,7 @@ define_machine(mpc832x_rdb) {
> .name   = "MPC832x RDB",
> .probe  = mpc832x_rdb_probe,
> .setup_arch = mpc832x_rdb_setup_arch,
> -   .init_IRQ   = mpc83xx_ipic_and_qe_init_IRQ,
> +   .init_IRQ   = mpc83xx_ipic_init_IRQ,
> .get_irq= ipic_get_irq,
> .restart= mpc83xx_restart,
> .time_init  = mpc83xx_time_init,
> diff --git a/arch/powerpc/platforms/83xx/mpc836x_mds.c 
> b/arch/powerpc/platforms/83xx/mpc836x_mds.c
> index 5b484da9533e..219a83ab6c00 100644
> --- a/arch/powerpc/platforms/83xx/mpc836x_mds.c
> +++ b/arch/powerpc/platforms/83xx/mpc836x_mds.c
> @@ -208,7 +208,7 @@ define_machine(mpc836x_mds) {
> .name   = "MPC836x MDS",
> .probe  = mpc836x_mds_probe,
> .setup_arch = mpc836x_mds_setup_arch,
> -   .init_IRQ   = mpc83xx_ipic_and_qe_init_IRQ,
> +   .init_IRQ   = mpc83xx_ipic_init_IRQ,
> .get_irq= ipic_get_irq,
> .restart= mpc83xx_restart,
> .time_init  = mpc83xx_time_init,
> diff --git a/arch/powerpc/platforms/83xx/mpc836x_rdk.c 
> b/arch/powerpc/platforms/83xx/mpc836x_rdk.c
> index b7119e443920..b4aac2cde849 100644
> --- a/arch/powerpc/platforms/83xx/mpc836x_rdk.c
> +++ b/arch/powerpc/platforms/83xx/mpc836x_rdk.c
> @@ -41,7 +41,7 @@ define_machine(mpc836x_rdk) {
> .name   = "MPC836x RDK",
> .probe  = mpc836x_rdk_probe,
> .setup_arch = mpc836x_rdk_setup_arch,
> -   .init_IRQ   = mpc83xx_ipic_and_qe_init_IRQ,
> +   .init_IRQ   = mpc83xx_ipic_init_IRQ,
> .get_irq= ipic_get_irq,
> .restart= mpc83xx_restart,
> .time_init  = mpc83xx_time_init,
> diff --git a/arch/powerpc/platforms/83xx/mpc83xx.h 
> b/arch/powerpc/platforms/83xx/mpc83xx.h
> index d343f6ce2599..f37d04332fc7 100644
> --- a/arch/powerpc/platforms/83xx/mpc83xx.h
> +++ 

Re: [PATCH v6 00/49] QUICC Engine support on ARM, ARM64, PPC64

[Li Yang]

On Mon, Dec 2, 2019 at 2:14 AM Rasmus Villemoes
 wrote:
>
> On 01/12/2019 17.10, Timur Tabi wrote:
> > On 11/28/19 8:55 AM, Rasmus Villemoes wrote:
> >> There have been several attempts in the past few years to allow
> >> building the QUICC engine drivers for platforms other than PPC32. This
> >> is yet another attempt.
> >>
> >> v5 can be found
> >> here:https://lore.kernel.org/lkml/20191118112324.22725-1-li...@rasmusvillemoes.dk/
> >>
> >
> > If it helps:
> >
> > Entire series:
> > Acked-by: Timur Tabi 
>
> Thanks. I'll leave it to Li Yang whether to apply that - they already
> all (except for the last-minute build fix) have your R-b.
>
> Li Yang, any chance you could pick up these patches so they have plenty
> of time in -next until 5.6?

Sure.  I will.  I'm waiting for the Ack from David on the networking side.

Regards,
Leo

Re: [v3, 3/3] Documentation: dt: binding: fsl: Add 'fsl, ippdexpcr-alt-addr' property

[Li Yang]

On Tue, Sep 24, 2019 at 11:27 PM Biwen Li  wrote:
>
> > > >
> > > > > > > > > >
> > > > > > > > > > The 'fsl,ippdexpcr-alt-addr' property is used to handle
> > > > > > > > > > an errata
> > > > > > > > > > A-008646 on LS1021A
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Biwen Li 
> > > > > > > > > > ---
> > > > > > > > > > Change in v3:
> > > > > > > > > >   - rename property name
> > > > > > > > > > fsl,rcpm-scfg -> fsl,ippdexpcr-alt-addr
> > > > > > > > > >
> > > > > > > > > > Change in v2:
> > > > > > > > > >   - update desc of the property 'fsl,rcpm-scfg'
> > > > > > > > > >
> > > > > > > > > >  Documentation/devicetree/bindings/soc/fsl/rcpm.txt | 14
> > > > > > > > > > ++
> > > > > > > > > >  1 file changed, 14 insertions(+)
> > > > > > > > > >
> > > > > > > > > > diff --git
> > > > > > > > > > a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > > > > > > b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > > > > > > index 5a33619d881d..157dcf6da17c 100644
> > > > > > > > > > --- a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > > > > > > +++ b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > > > > > > @@ -34,6 +34,11 @@ Chassis VersionExample
> > > > Chips
> > > > > > > > > >  Optional properties:
> > > > > > > > > >   - little-endian : RCPM register block is Little Endian.
> > > > > > > > > > Without it
> > > > > RCPM
> > > > > > > > > > will be Big Endian (default case).
> > > > > > > > > > + - fsl,ippdexpcr-alt-addr : Must add the property for
> > > > > > > > > > + SoC LS1021A,
> > > > > > > > >
> > > > > > > > > You probably should mention this is related to a hardware
> > > > > > > > > issue on LS1021a and only needed on LS1021a.
> > > > > > > > Okay, got it, thanks, I will add this in v4.
> > > > > > > > >
> > > > > > > > > > +   Must include n + 1 entries (n =
> > > > > > > > > > + #fsl,rcpm-wakeup-cells, such
> > > as:
> > > > > > > > > > +   #fsl,rcpm-wakeup-cells equal to 2, then must include
> > > > > > > > > > + 2
> > > > > > > > > > + +
> > > > > > > > > > + 1
> > > > > entries).
> > > > > > > > >
> > > > > > > > > #fsl,rcpm-wakeup-cells is the number of IPPDEXPCR
> > > > > > > > > registers on an
> > > > > SoC.
> > > > > > > > > However you are defining an offset to scfg registers here.
> > > > > > > > > Why these two are related?  The length here should
> > > > > > > > > actually be related to the #address-cells of the soc/.
> > > > > > > > > But since this is only needed for LS1021, you can
> > > > > > > > just make it 3.
> > > > > > > > I need set the value of IPPDEXPCR resgiters from ftm_alarm0
> > > > > > > > device node(fsl,rcpm-wakeup = < 0x0 0x2000>;
> > > > > > > > 0x0 is a value for IPPDEXPCR0, 0x2000 is a value for
> > > > > IPPDEXPCR1).
> > > > > > > > But because of the hardware issue on LS1021A, I need store
> > > > > > > > the value of IPPDEXPCR registers to an alt address. So I
> > > > > > > > defining an offset to scfg registers, then RCPM driver get
> > > > > > > > an abosolute address from offset, RCPM driver write the
> > > > > > > > value of IPPDEXPCR registers to these abosolute
> > > > > > > > addresses(backup the value of IPPDEXPCR
> > > > > registers).
> > > > > > >
> > > > > > > I understand what you are trying to do.  The problem is that
> > > > > > > the new fsl,ippdexpcr-alt-addr property contains a phandle and an
> > offset.
> > > > > > > The size of it shouldn't be related to #fsl,rcpm-wakeup-cells.
> > > > > > You maybe like this: fsl,ippdexpcr-alt-addr = < 0x51c>;/*
> > > > > > SCFG_SPARECR8 */
> > > > >
> > > > > No.  The #address-cell for the soc/ is 2, so the offset to scfg
> > > > > should be 0x0 0x51c.  The total size should be 3, but it shouldn't
> > > > > be coming from #fsl,rcpm-wakeup-cells like you mentioned in the
> > binding.
> > > > Oh, I got it. You want that fsl,ippdexpcr-alt-add is relative with
> > > > #address-cells instead of #fsl,rcpm-wakeup-cells.
> > >
> > > Yes.
> > I got an example from drivers/pci/controller/dwc/pci-layerscape.c
> > and arch/arm/boot/dts/ls1021a.dtsi as follows:
> > fsl,pcie-scfg = < 0>, 0 is an index
> >
> > In my fsl,ippdexpcr-alt-addr = < 0x0 0x51c>, It means that 0x0 is an 
> > alt
> > offset address for IPPDEXPCR0, 0x51c is an alt offset address For
> > IPPDEXPCR1 instead of 0x0 and 0x51c compose to an alt address of
> > SCFG_SPARECR8.
> Maybe I need write it as:
> fsl,ippdexpcr-alt-addr = < 0x0 0x0 0x0 0x51c>;
> first two 0x0 compose an alt offset address for IPPDEXPCR0,
> last 0x0 and 0x51c compose an alt address for IPPDEXPCR1,

I remember the hardware issue is only is only related to IPPDEXPCR1
register, no idea why you need to define IPPDEXPCR0 in the binding.

>
> Best Regards,
> Biwen Li
> > >
> > > Regards,
> > > Leo
> > > > >
> > > > > > >
> > > > > > > > >
> > > > > > > > > > +   The first entry must be a link to the SCFG device node.
> > > > > > > > > > +   The non-first entry must be 

Re: [PATCH 0/3] soc: fsl: dpio: Enable QMAN batch enqueuing

[Li Yang]

On Thu, Feb 6, 2020 at 2:41 PM Roy Pledge  wrote:
>
> On 12/12/2019 12:01 PM, Youri Querry wrote:
> > This patch set consists of:
> > - We added an interface to enqueue several packets at a time and
> >improve performance.
> > - Make the algorithm decisions once at initialization and use
> >function pointers to improve performance.
> > - Replaced the QMAN enqueue array mode algorithm with a ring
> >mode algorithm. This is to make the enqueue of several frames
> >at a time more effective.
> >
> > Youri Querry (3):
> >soc: fsl: dpio: Adding QMAN multiple enqueue interface.
> >soc: fsl: dpio: QMAN performance improvement. Function pointer
> >  indirection.
> >soc: fsl: dpio: Replace QMAN array mode by ring mode enqueue.
> >
> >   drivers/soc/fsl/dpio/dpio-service.c |  69 +++-
> >   drivers/soc/fsl/dpio/qbman-portal.c | 766 
> > 
> >   drivers/soc/fsl/dpio/qbman-portal.h | 158 +++-
> >   include/soc/fsl/dpaa2-io.h  |   6 +-
> >   4 files changed, 907 insertions(+), 92 deletions(-)
> >
> Acked-by: Roy Pledge 

Series applied with some clean up and typo fix in the title/commit message.

Regards,
Leo

Re: [PATCH -next] soc: fsl: qe: remove set but not used variable 'mm_gc'

[Li Yang]

On Wed, Jan 8, 2020 at 7:12 AM YueHaibing  wrote:
>
> drivers/soc/fsl/qe/gpio.c: In function qe_pin_request:
> drivers/soc/fsl/qe/gpio.c:163:26: warning: variable mm_gc set but not used 
> [-Wunused-but-set-variable]
>
> commit 1e714e54b5ca ("powerpc: qe_lib-gpio: use gpiochip data pointer")
> left behind this unused variable.
>
> Reported-by: Hulk Robot 
> Signed-off-by: YueHaibing 

Applied for next.  Thanks.

Btw, I find another patch from Chen Zhou fixing the same problem sent
earlier.  I will add his signed-off-by to the commit for credit too.

Regards,
Leo

> ---
>  drivers/soc/fsl/qe/gpio.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/drivers/soc/fsl/qe/gpio.c b/drivers/soc/fsl/qe/gpio.c
> index 12bdfd9..ed75198 100644
> --- a/drivers/soc/fsl/qe/gpio.c
> +++ b/drivers/soc/fsl/qe/gpio.c
> @@ -160,7 +160,6 @@ struct qe_pin *qe_pin_request(struct device_node *np, int 
> index)
>  {
> struct qe_pin *qe_pin;
> struct gpio_chip *gc;
> -   struct of_mm_gpio_chip *mm_gc;
> struct qe_gpio_chip *qe_gc;
> int err;
> unsigned long flags;
> @@ -186,7 +185,6 @@ struct qe_pin *qe_pin_request(struct device_node *np, int 
> index)
> goto err0;
> }
>
> -   mm_gc = to_of_mm_gpio_chip(gc);
> qe_gc = gpiochip_get_data(gc);
>
> spin_lock_irqsave(_gc->lock, flags);
> --
> 2.7.4
>
>

Re: [PATCH -next] soc: fsl: dpio: remove set but not used variable 'addr_cena'

[Li Yang]

On Fri, Feb 21, 2020 at 2:38 AM YueHaibing  wrote:
>
> commit 3b2abda7d28c ("soc: fsl: dpio: Replace QMAN array
> mode with ring mode enqueue") introduced this, but not
> used, so remove it.
>
> Reported-by: Hulk Robot 
> Signed-off-by: YueHaibing 
> ---
>  drivers/soc/fsl/dpio/qbman-portal.c | 4 
>  1 file changed, 4 deletions(-)
>
> diff --git a/drivers/soc/fsl/dpio/qbman-portal.c 
> b/drivers/soc/fsl/dpio/qbman-portal.c
> index 740ee0d..350de56 100644
> --- a/drivers/soc/fsl/dpio/qbman-portal.c
> +++ b/drivers/soc/fsl/dpio/qbman-portal.c
> @@ -658,7 +658,6 @@ int qbman_swp_enqueue_multiple_direct(struct qbman_swp *s,
> const uint32_t *cl = (uint32_t *)d;
> uint32_t eqcr_ci, eqcr_pi, half_mask, full_mask;
> int i, num_enqueued = 0;
> -   uint64_t addr_cena;
>
> spin_lock(>access_spinlock);
> half_mask = (s->eqcr.pi_ci_mask>>1);
> @@ -711,7 +710,6 @@ int qbman_swp_enqueue_multiple_direct(struct qbman_swp *s,
>
> /* Flush all the cacheline without load/store in between */
> eqcr_pi = s->eqcr.pi;
> -   addr_cena = (size_t)s->addr_cena;
> for (i = 0; i < num_enqueued; i++)
> eqcr_pi++;
> s->eqcr.pi = eqcr_pi & full_mask;
> @@ -822,7 +820,6 @@ int qbman_swp_enqueue_multiple_desc_direct(struct 
> qbman_swp *s,
> const uint32_t *cl;
> uint32_t eqcr_ci, eqcr_pi, half_mask, full_mask;
> int i, num_enqueued = 0;
> -   uint64_t addr_cena;
>
> half_mask = (s->eqcr.pi_ci_mask>>1);
> full_mask = s->eqcr.pi_ci_mask;
> @@ -866,7 +863,6 @@ int qbman_swp_enqueue_multiple_desc_direct(struct 
> qbman_swp *s,
>
> /* Flush all the cacheline without load/store in between */
> eqcr_pi = s->eqcr.pi;
> -   addr_cena = (uint64_t)s->addr_cena;

Hi Youri,

Looks like this problem exposed another issue that you removed the
cacheline related code in the upstream version.  Then the comment /*
Flush all the cacheline without load/store in between */ is no longer
true now, and probably the whole block can be replaced with a single
line to increase s->eqcr.pi?   The same for the block above.  Can you
provide a better fix for this issue?

Regards,
Leo

> for (i = 0; i < num_enqueued; i++)
> eqcr_pi++;
> s->eqcr.pi = eqcr_pi & full_mask;
> --
> 2.7.4
>
>

[PATCH 5/6] soc: fsl: qe: fix sparse warnings for ucc_fast.c

[Li Yang]

Fixes the following sparse warnings:

drivers/soc/fsl/qe/ucc_fast.c:218:22: warning: incorrect type in assignment 
(different base types)
drivers/soc/fsl/qe/ucc_fast.c:218:22:expected unsigned int [noderef] 
[usertype]  *p_ucce
drivers/soc/fsl/qe/ucc_fast.c:218:22:got restricted __be32 [noderef] 
 *
drivers/soc/fsl/qe/ucc_fast.c:219:22: warning: incorrect type in assignment 
(different base types)
drivers/soc/fsl/qe/ucc_fast.c:219:22:expected unsigned int [noderef] 
[usertype]  *p_uccm
drivers/soc/fsl/qe/ucc_fast.c:219:22:got restricted __be32 [noderef] 
 *

Signed-off-by: Li Yang 
---
 include/soc/fsl/qe/ucc_fast.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/soc/fsl/qe/ucc_fast.h b/include/soc/fsl/qe/ucc_fast.h
index ba0e838f962a..dc4e79468094 100644
--- a/include/soc/fsl/qe/ucc_fast.h
+++ b/include/soc/fsl/qe/ucc_fast.h
@@ -178,10 +178,10 @@ struct ucc_fast_info {
 struct ucc_fast_private {
struct ucc_fast_info *uf_info;
struct ucc_fast __iomem *uf_regs; /* a pointer to the UCC regs. */
-   u32 __iomem *p_ucce;/* a pointer to the event register in memory. */
-   u32 __iomem *p_uccm;/* a pointer to the mask register in memory. */
+   __be32 __iomem *p_ucce; /* a pointer to the event register in memory. */
+   __be32 __iomem *p_uccm; /* a pointer to the mask register in memory. */
 #ifdef CONFIG_UGETH_TX_ON_DEMAND
-   u16 __iomem *p_utodr;   /* pointer to the transmit on demand register */
+   __be16 __iomem *p_utodr;/* pointer to the transmit on demand register */
 #endif
int enabled_tx; /* Whether channel is enabled for Tx (ENT) */
int enabled_rx; /* Whether channel is enabled for Rx (ENR) */
-- 
2.17.1

[PATCH 1/6] soc: fsl: qe: fix sparse warnings for qe.c

[Li Yang]

Fixes the following sparse warnings:
drivers/soc/fsl/qe/qe.c:426:9: warning: cast to restricted __be32
drivers/soc/fsl/qe/qe.c:528:41: warning: incorrect type in assignment 
(different base types)
drivers/soc/fsl/qe/qe.c:528:41:expected unsigned long long static 
[addressable] [toplevel] [usertype] extended_modes
drivers/soc/fsl/qe/qe.c:528:41:got restricted __be64 const [usertype] 
extended_modes

Signed-off-by: Li Yang 
---
 drivers/soc/fsl/qe/qe.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/fsl/qe/qe.c b/drivers/soc/fsl/qe/qe.c
index 96c2057d8d8e..447146861c2c 100644
--- a/drivers/soc/fsl/qe/qe.c
+++ b/drivers/soc/fsl/qe/qe.c
@@ -423,7 +423,7 @@ static void qe_upload_microcode(const void *base,
qe_iowrite32be(be32_to_cpu(code[i]), _immr->iram.idata);

/* Set I-RAM Ready Register */
-   qe_iowrite32be(be32_to_cpu(QE_IRAM_READY), _immr->iram.iready);
+   qe_iowrite32be(QE_IRAM_READY, _immr->iram.iready);
 }
 
 /*
@@ -525,7 +525,7 @@ int qe_upload_firmware(const struct qe_firmware *firmware)
 */
memset(_firmware_info, 0, sizeof(qe_firmware_info));
strlcpy(qe_firmware_info.id, firmware->id, sizeof(qe_firmware_info.id));
-   qe_firmware_info.extended_modes = firmware->extended_modes;
+   qe_firmware_info.extended_modes = be64_to_cpu(firmware->extended_modes);
memcpy(qe_firmware_info.vtraps, firmware->vtraps,
sizeof(firmware->vtraps));
 
-- 
2.17.1

[PATCH 2/6] soc: fsl: qe: fix sparse warning for qe_common.c

[Li Yang]

Fixes the following sparse warning:

drivers/soc/fsl/qe/qe_common.c:75:48: warning: incorrect type in argument 2 
(different base types)
drivers/soc/fsl/qe/qe_common.c:75:48:expected restricted __be32 const 
[usertype] *addr
drivers/soc/fsl/qe/qe_common.c:75:48:got unsigned int *

Signed-off-by: Li Yang 
---
 drivers/soc/fsl/qe/qe_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soc/fsl/qe/qe_common.c b/drivers/soc/fsl/qe/qe_common.c
index a81a1a79f1ca..75075591f630 100644
--- a/drivers/soc/fsl/qe/qe_common.c
+++ b/drivers/soc/fsl/qe/qe_common.c
@@ -46,7 +46,7 @@ int cpm_muram_init(void)
 {
struct device_node *np;
struct resource r;
-   u32 zero[OF_MAX_ADDR_CELLS] = {};
+   __be32 zero[OF_MAX_ADDR_CELLS] = {};
resource_size_t max = 0;
int i = 0;
int ret = 0;
-- 
2.17.1

[PATCH 4/6] soc: fsl: qe: fix sparse warnings for qe_ic.c

[Li Yang]

Fixes the following sparse warnings:

drivers/soc/fsl/qe/qe_ic.c:253:32: warning: incorrect type in argument 1 
(different base types)
drivers/soc/fsl/qe/qe_ic.c:253:32:expected restricted __be32 [noderef] 
[usertype]  *base
drivers/soc/fsl/qe/qe_ic.c:253:32:got unsigned int [noderef] [usertype] 
 *regs
drivers/soc/fsl/qe/qe_ic.c:254:26: warning: incorrect type in argument 1 
(different base types)
drivers/soc/fsl/qe/qe_ic.c:254:26:expected restricted __be32 [noderef] 
[usertype]  *base
drivers/soc/fsl/qe/qe_ic.c:254:26:got unsigned int [noderef] [usertype] 
 *regs
drivers/soc/fsl/qe/qe_ic.c:269:32: warning: incorrect type in argument 1 
(different base types)
drivers/soc/fsl/qe/qe_ic.c:269:32:expected restricted __be32 [noderef] 
[usertype]  *base
drivers/soc/fsl/qe/qe_ic.c:269:32:got unsigned int [noderef] [usertype] 
 *regs
drivers/soc/fsl/qe/qe_ic.c:270:26: warning: incorrect type in argument 1 
(different base types)
drivers/soc/fsl/qe/qe_ic.c:270:26:expected restricted __be32 [noderef] 
[usertype]  *base
drivers/soc/fsl/qe/qe_ic.c:270:26:got unsigned int [noderef] [usertype] 
 *regs
drivers/soc/fsl/qe/qe_ic.c:341:31: warning: incorrect type in argument 1 
(different base types)
drivers/soc/fsl/qe/qe_ic.c:341:31:expected restricted __be32 [noderef] 
[usertype]  *base
drivers/soc/fsl/qe/qe_ic.c:341:31:got unsigned int [noderef] [usertype] 
 *regs
drivers/soc/fsl/qe/qe_ic.c:357:31: warning: incorrect type in argument 1 
(different base types)
drivers/soc/fsl/qe/qe_ic.c:357:31:expected restricted __be32 [noderef] 
[usertype]  *base
drivers/soc/fsl/qe/qe_ic.c:357:31:got unsigned int [noderef] [usertype] 
 *regs
drivers/soc/fsl/qe/qe_ic.c:450:26: warning: incorrect type in argument 1 
(different base types)
drivers/soc/fsl/qe/qe_ic.c:450:26:expected restricted __be32 [noderef] 
[usertype]  *base
drivers/soc/fsl/qe/qe_ic.c:450:26:got unsigned int [noderef] [usertype] 
 *regs

Signed-off-by: Li Yang 
---
 drivers/soc/fsl/qe/qe_ic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soc/fsl/qe/qe_ic.c b/drivers/soc/fsl/qe/qe_ic.c
index 0dd5bdb04a14..0390af00 100644
--- a/drivers/soc/fsl/qe/qe_ic.c
+++ b/drivers/soc/fsl/qe/qe_ic.c
@@ -44,7 +44,7 @@
 
 struct qe_ic {
/* Control registers offset */
-   u32 __iomem *regs;
+   __be32 __iomem *regs;
 
/* The remapper for this QEIC */
struct irq_domain *irqhost;
-- 
2.17.1

[PATCH 3/6] soc: fsl: qe: fix sparse warnings for ucc.c

[Li Yang]

Fixes the following sparse warnings:

drivers/soc/fsl/qe/ucc.c:637:20: warning: incorrect type in assignment 
(different address spaces)
drivers/soc/fsl/qe/ucc.c:637:20:expected struct qe_mux *qe_mux_reg
drivers/soc/fsl/qe/ucc.c:637:20:got struct qe_mux [noderef]  *
drivers/soc/fsl/qe/ucc.c:652:9: warning: incorrect type in argument 1 
(different address spaces)
drivers/soc/fsl/qe/ucc.c:652:9:expected void const volatile [noderef] 
 *addr
drivers/soc/fsl/qe/ucc.c:652:9:got restricted __be32 *
drivers/soc/fsl/qe/ucc.c:652:9: warning: incorrect type in argument 2 
(different address spaces)
drivers/soc/fsl/qe/ucc.c:652:9:expected void volatile [noderef]  
*addr
drivers/soc/fsl/qe/ucc.c:652:9:got restricted __be32 *

Signed-off-by: Li Yang 
---
 drivers/soc/fsl/qe/ucc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soc/fsl/qe/ucc.c b/drivers/soc/fsl/qe/ucc.c
index 90157acc5ba6..d6c93970df4d 100644
--- a/drivers/soc/fsl/qe/ucc.c
+++ b/drivers/soc/fsl/qe/ucc.c
@@ -632,7 +632,7 @@ int ucc_set_tdm_rxtx_sync(u32 tdm_num, enum qe_clock clock,
 {
int source;
u32 shift;
-   struct qe_mux *qe_mux_reg;
+   struct qe_mux __iomem *qe_mux_reg;
 
qe_mux_reg = _immr->qmx;
 
-- 
2.17.1

[PATCH 0/6] Fix sparse warnings for common qe library code

[Li Yang]

The QE code was previously only supported on big-endian PowerPC systems
that use the same endian as the QE device.  The endian transfer code is
not really exercised.  Recent updates extended the QE drivers to
little-endian ARM/ARM64 systems which makes the endian transfer really
meaningful and hence triggered more sparse warnings for the endian
mismatch.  Some of these endian issues are real issues that need to be
fixed.

While at it, fixed some direct de-references of IO memory space and
suppressed other __iomem address space mismatch issues by adding correct
address space attributes.

Li Yang (6):
  soc: fsl: qe: fix sparse warnings for qe.c
  soc: fsl: qe: fix sparse warning for qe_common.c
  soc: fsl: qe: fix sparse warnings for ucc.c
  soc: fsl: qe: fix sparse warnings for qe_ic.c
  soc: fsl: qe: fix sparse warnings for ucc_fast.c
  soc: fsl: qe: fix sparse warnings for ucc_slow.c

 drivers/soc/fsl/qe/qe.c|  4 ++--
 drivers/soc/fsl/qe/qe_common.c |  2 +-
 drivers/soc/fsl/qe/qe_ic.c |  2 +-
 drivers/soc/fsl/qe/ucc.c   |  2 +-
 drivers/soc/fsl/qe/ucc_slow.c  | 33 +
 include/soc/fsl/qe/ucc_fast.h  |  6 +++---
 include/soc/fsl/qe/ucc_slow.h  | 13 ++---
 7 files changed, 27 insertions(+), 35 deletions(-)

-- 
2.17.1

[PATCH 6/6] soc: fsl: qe: fix sparse warnings for ucc_slow.c

[Li Yang]

:got restricted __be32 [usertype]
drivers/soc/fsl/qe/ucc_slow.c:251:9: warning: cast from restricted __be32
drivers/soc/fsl/qe/ucc_slow.c:251:9: warning: incorrect type in argument 2 
(different address spaces)
drivers/soc/fsl/qe/ucc_slow.c:251:9:expected void volatile [noderef] 
 *addr
drivers/soc/fsl/qe/ucc_slow.c:251:9:got unsigned int [usertype] *
drivers/soc/fsl/qe/ucc_slow.c:252:9: warning: incorrect type in argument 2 
(different address spaces)
drivers/soc/fsl/qe/ucc_slow.c:252:9:expected void volatile [noderef] 
 *addr
drivers/soc/fsl/qe/ucc_slow.c:252:9:got restricted __be32 *
drivers/soc/fsl/qe/ucc_slow.c:276:39: warning: mixing different enum types:
drivers/soc/fsl/qe/ucc_slow.c:276:39:unsigned int enum 
ucc_slow_tx_oversampling_rate
drivers/soc/fsl/qe/ucc_slow.c:276:39:unsigned int enum 
ucc_slow_rx_oversampling_rate
drivers/soc/fsl/qe/ucc_slow.c:296:9: warning: incorrect type in argument 2 
(different address spaces)
drivers/soc/fsl/qe/ucc_slow.c:296:9:expected void volatile [noderef] 
 *addr
drivers/soc/fsl/qe/ucc_slow.c:296:9:got restricted __be16 *
drivers/soc/fsl/qe/ucc_slow.c:297:9: warning: incorrect type in argument 2 
(different address spaces)
drivers/soc/fsl/qe/ucc_slow.c:297:9:expected void volatile [noderef] 
 *addr
drivers/soc/fsl/qe/ucc_slow.c:297:9:got restricted __be16 *

Also removed the unneccessary clearing for kzalloc'ed structure.

Signed-off-by: Li Yang 
---
 drivers/soc/fsl/qe/ucc_slow.c | 33 +
 include/soc/fsl/qe/ucc_slow.h | 13 ++---
 2 files changed, 19 insertions(+), 27 deletions(-)

diff --git a/drivers/soc/fsl/qe/ucc_slow.c b/drivers/soc/fsl/qe/ucc_slow.c
index 274d34449846..7e11be41ab62 100644
--- a/drivers/soc/fsl/qe/ucc_slow.c
+++ b/drivers/soc/fsl/qe/ucc_slow.c
@@ -72,7 +72,7 @@ EXPORT_SYMBOL(ucc_slow_restart_tx);
 
 void ucc_slow_enable(struct ucc_slow_private * uccs, enum comm_dir mode)
 {
-   struct ucc_slow *us_regs;
+   struct ucc_slow __iomem *us_regs;
u32 gumr_l;
 
us_regs = uccs->us_regs;
@@ -93,7 +93,7 @@ EXPORT_SYMBOL(ucc_slow_enable);
 
 void ucc_slow_disable(struct ucc_slow_private * uccs, enum comm_dir mode)
 {
-   struct ucc_slow *us_regs;
+   struct ucc_slow __iomem *us_regs;
u32 gumr_l;
 
us_regs = uccs->us_regs;
@@ -122,7 +122,7 @@ int ucc_slow_init(struct ucc_slow_info * us_info, struct 
ucc_slow_private ** ucc
u32 i;
struct ucc_slow __iomem *us_regs;
u32 gumr;
-   struct qe_bd *bd;
+   struct qe_bd __iomem *bd;
u32 id;
u32 command;
int ret = 0;
@@ -168,16 +168,9 @@ int ucc_slow_init(struct ucc_slow_info * us_info, struct 
ucc_slow_private ** ucc
return -ENOMEM;
}
 
-   uccs->saved_uccm = 0;
-   uccs->p_rx_frame = 0;
us_regs = uccs->us_regs;
-   uccs->p_ucce = (u16 *) & (us_regs->ucce);
-   uccs->p_uccm = (u16 *) & (us_regs->uccm);
-#ifdef STATISTICS
-   uccs->rx_frames = 0;
-   uccs->tx_frames = 0;
-   uccs->rx_discarded = 0;
-#endif /* STATISTICS */
+   uccs->p_ucce = _regs->ucce;
+   uccs->p_uccm = _regs->uccm;
 
/* Get PRAM base */
uccs->us_pram_offset =
@@ -231,24 +224,24 @@ int ucc_slow_init(struct ucc_slow_info * us_info, struct 
ucc_slow_private ** ucc
/* clear bd buffer */
qe_iowrite32be(0, >buf);
/* set bd status and length */
-   qe_iowrite32be(0, (u32 *)bd);
+   qe_iowrite32be(0, (u32 __iomem *)bd);
bd++;
}
/* for last BD set Wrap bit */
qe_iowrite32be(0, >buf);
-   qe_iowrite32be(cpu_to_be32(T_W), (u32 *)bd);
+   qe_iowrite32be(T_W, (u32 __iomem *)bd);
 
/* Init Rx bds */
bd = uccs->rx_bd = qe_muram_addr(uccs->rx_base_offset);
for (i = 0; i < us_info->rx_bd_ring_len - 1; i++) {
/* set bd status and length */
-   qe_iowrite32be(0, (u32 *)bd);
+   qe_iowrite32be(0, (u32 __iomem *)bd);
/* clear bd buffer */
qe_iowrite32be(0, >buf);
bd++;
}
/* for last BD set Wrap bit */
-   qe_iowrite32be(cpu_to_be32(R_W), (u32 *)bd);
+   qe_iowrite32be(R_W, (u32 __iomem *)bd);
qe_iowrite32be(0, >buf);
 
/* Set GUMR (For more details see the hardware spec.). */
@@ -273,8 +266,8 @@ int ucc_slow_init(struct ucc_slow_info * us_info, struct 
ucc_slow_private ** ucc
qe_iowrite32be(gumr, _regs->gumr_h);
 
/* gumr_l */
-   gumr = us_info->tdcr | us_info->rdcr | us_info->tenc | us_info->renc |
-   us_info->diag | us_info->mode;
+   gumr = (u32)us_info->tdcr | (u32)us_info->rdcr | (u32)us_info->tenc |
+  (u32)us_info->renc | (u32)us_info-&g

Re: usb: gadget: fsl_udc_core: Checking for a failed platform_get_irq() call in fsl_udc_probe()

[Li Yang]

On Wed, Apr 8, 2020 at 9:19 AM Markus Elfring  wrote:
>
> Hello,
>
> I have taken another look at the implementation of the function 
> “fsl_udc_probe”.
> A software analysis approach points the following source code out for
> further development considerations.
> https://elixir.bootlin.com/linux/v5.6.2/source/drivers/usb/gadget/udc/fsl_udc_core.c#L2443
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/gadget/udc/fsl_udc_core.c?id=f5e94d10e4c468357019e5c28d48499f677b284f#n2442
>
> udc_controller->irq = platform_get_irq(pdev, 0);
> if (!udc_controller->irq) {
> ret = -ENODEV;
> goto err_iounmap;
> }
>
>
> The software documentation is providing the following information
> for the used programming interface.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/base/platform.c?id=f5e94d10e4c468357019e5c28d48499f677b284f#n221
> https://elixir.bootlin.com/linux/v5.6.2/source/drivers/base/platform.c#L202
>
> “…
>  * Return: IRQ number on success, negative error number on failure.
> …”
>
> Would you like to reconsider the shown condition check?

Thanks for the finding.  This is truly a software issue that need to
be fixed.  Would you submit a patch for it or you want us to fix it?

Regards,
Leo

Re: [PATCH 6/6] soc: fsl: qe: fix sparse warnings for ucc_slow.c

[Li Yang]

On Mon, Mar 16, 2020 at 4:08 PM Rasmus Villemoes
 wrote:
>
> On 12/03/2020 23.28, Li Yang wrote:
> > Fixes the following sparse warnings:
> >
> [snip]
> >
> > Also removed the unneccessary clearing for kzalloc'ed structure.
>
> Please don't mix that in the same patch, do it in a preparatory patch.
> That makes reviewing much easier.

Thanks for the review.

One of the few lines removed are actually causing sparse warning,
that's why I changed this together with the sparse fixes.  I can add
all the lines removed in the log for better record.

>
> >
> >   /* Get PRAM base */
> >   uccs->us_pram_offset =
> > @@ -231,24 +224,24 @@ int ucc_slow_init(struct ucc_slow_info * us_info, 
> > struct ucc_slow_private ** ucc
> >   /* clear bd buffer */
> >   qe_iowrite32be(0, >buf);
> >   /* set bd status and length */
> > - qe_iowrite32be(0, (u32 *)bd);
> > + qe_iowrite32be(0, (u32 __iomem *)bd);
>
> It's cleaner to do two qe_iowrite16be to >status and >length,
> that avoids the casting altogether.

It probably is cleaner, but also slower for the bd manipulation that
can be in the hot path.  The QE code has been using this way to
access/update the bd status and length together for a long time.  And
I remembered that it could help to prevent synchronization issues for
accessing status and length separately.

It is probably cleaner to change the data structure to use union for
accessing status and length together.  But that will need a big change
to update all the current users of the structure which I don't have
the time to do right now.

>
> >   bd++;
> >   }
> >   /* for last BD set Wrap bit */
> >   qe_iowrite32be(0, >buf);
> > - qe_iowrite32be(cpu_to_be32(T_W), (u32 *)bd);
> > + qe_iowrite32be(T_W, (u32 __iomem *)bd);
>
> Yeah, and this is why. Who can actually keep track of where that bit
> ends up being set with that casting going on. Please use
> qe_iowrite16be() with an appropriately modified constant to the
> appropriate field instead of these games.
>
> And if the hardware doesn't support 16 bit writes, the definition of
> struct qe_bd is wrong and should have a single __be32 status_length
> field, with appropriate accessors defined.

Same comment as above.

>
> >   /* Init Rx bds */
> >   bd = uccs->rx_bd = qe_muram_addr(uccs->rx_base_offset);
> >   for (i = 0; i < us_info->rx_bd_ring_len - 1; i++) {
> >   /* set bd status and length */
> > - qe_iowrite32be(0, (u32 *)bd);
> > + qe_iowrite32be(0, (u32 __iomem *)bd);
>
> Same.
>
> >   /* clear bd buffer */
> >   qe_iowrite32be(0, >buf);
> >   bd++;
> >   }
> >   /* for last BD set Wrap bit */
> > - qe_iowrite32be(cpu_to_be32(R_W), (u32 *)bd);
> > + qe_iowrite32be(R_W, (u32 __iomem *)bd);
>
> Same.
>
> >   qe_iowrite32be(0, >buf);
> >
> >   /* Set GUMR (For more details see the hardware spec.). */
> > @@ -273,8 +266,8 @@ int ucc_slow_init(struct ucc_slow_info * us_info, 
> > struct ucc_slow_private ** ucc
> >   qe_iowrite32be(gumr, _regs->gumr_h);
> >
> >   /* gumr_l */
> > - gumr = us_info->tdcr | us_info->rdcr | us_info->tenc | us_info->renc |
> > - us_info->diag | us_info->mode;
> > + gumr = (u32)us_info->tdcr | (u32)us_info->rdcr | (u32)us_info->tenc |
> > +(u32)us_info->renc | (u32)us_info->diag | (u32)us_info->mode;
>
> Are the tdcr, rdcr, tenc, renc fields actually set anywhere (the same
> for the diag and mode, but word-grepping for those give way too many
> false positives)? They seem to be a somewhat pointless split out of the
> bitfields of gumr_l, and not populated anywhere?. That's not directly
> related to this patch, of course, but getting rid of them first (if they
> are indeed completely unused) might make the sparse cleanup a little
> simpler.

I would agree with you that is not neccessary to create different enum
types for these bits in the same register in the first place.  But I
would like to prevent aggressive cleanups of this driver for old
hardware that is becoming harder and harder to be thoroughly tested
right now.  These fields are probably not used by the in tree ucc_uart
driver right now.  But as a generic library for QE, I think it is
harmless to keep these simple code there for potentially customized
version of the uart driver or other ucc slow drivers.

Regards,
Leo

Re: [PATCH] soc: fsl: qe: Replace one-element array and use struct_size() helper

[Li Yang]

On Mon, May 18, 2020 at 5:57 PM Kees Cook  wrote:
>
> On Mon, May 18, 2020 at 05:19:04PM -0500, Gustavo A. R. Silva wrote:
> > The current codebase makes use of one-element arrays in the following
> > form:
> >
> > struct something {
> > int length;
> > u8 data[1];
> > };
> >
> > struct something *instance;
> >
> > instance = kmalloc(sizeof(*instance) + size, GFP_KERNEL);
> > instance->length = size;
> > memcpy(instance->data, source, size);
> >
> > but the preferred mechanism to declare variable-length types such as
> > these ones is a flexible array member[1][2], introduced in C99:
> >
> > struct foo {
> > int stuff;
> > struct boo array[];
> > };
> >
> > By making use of the mechanism above, we will get a compiler warning
> > in case the flexible array does not occur last in the structure, which
> > will help us prevent some kind of undefined behavior bugs from being
> > inadvertently introduced[3] to the codebase from now on. So, replace
> > the one-element array with a flexible-array member.
> >
> > Also, make use of the new struct_size() helper to properly calculate the
> > size of struct qe_firmware.
> >
> > This issue was found with the help of Coccinelle and, audited and fixed
> > _manually_.
> >
> > [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
> > [2] https://github.com/KSPP/linux/issues/21
> > [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
> >
> > Signed-off-by: Gustavo A. R. Silva 
> > ---
> >  drivers/soc/fsl/qe/qe.c | 4 ++--
> >  include/soc/fsl/qe/qe.h | 2 +-
> >  2 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/soc/fsl/qe/qe.c b/drivers/soc/fsl/qe/qe.c
> > index 447146861c2c1..2df20d6f85fa4 100644
> > --- a/drivers/soc/fsl/qe/qe.c
> > +++ b/drivers/soc/fsl/qe/qe.c
> > @@ -448,7 +448,7 @@ int qe_upload_firmware(const struct qe_firmware 
> > *firmware)
> >   unsigned int i;
> >   unsigned int j;
> >   u32 crc;
> > - size_t calc_size = sizeof(struct qe_firmware);
> > + size_t calc_size;
> >   size_t length;
> >   const struct qe_header *hdr;
> >
> > @@ -480,7 +480,7 @@ int qe_upload_firmware(const struct qe_firmware 
> > *firmware)
> >   }
> >
> >   /* Validate the length and check if there's a CRC */
> > - calc_size += (firmware->count - 1) * sizeof(struct qe_microcode);
> > + calc_size = struct_size(firmware, microcode, firmware->count);
> >
> >   for (i = 0; i < firmware->count; i++)
> >   /*
> > diff --git a/include/soc/fsl/qe/qe.h b/include/soc/fsl/qe/qe.h
> > index e282ac01ec081..3feddfec9f87d 100644
> > --- a/include/soc/fsl/qe/qe.h
> > +++ b/include/soc/fsl/qe/qe.h
> > @@ -307,7 +307,7 @@ struct qe_firmware {
> >   u8 revision;/* The microcode version revision */
> >   u8 padding; /* Reserved, for alignment */
> >   u8 reserved[4]; /* Reserved, for future expansion */
> > - } __attribute__ ((packed)) microcode[1];
> > + } __packed microcode[];
> >   /* All microcode binaries should be located here */
> >   /* CRC32 should be located here, after the microcode binaries */
> >  } __attribute__ ((packed));
> > --
> > 2.26.2
> >
>
> Hm, looking at this code, I see a few other things that need to be
> fixed:
>
> 1) drivers/tty/serial/ucc_uart.c does not do a be32_to_cpu() conversion
>on the length test (understandably, a little-endian system has never run
>this code since it's ppc specific), but it's still wrong:
>
> if (firmware->header.length != fw->size) {
>
>compare to the firmware loader:
>
> length = be32_to_cpu(hdr->length);
>
> 2) drivers/soc/fsl/qe/qe.c does not perform bounds checking on the
>per-microcode offsets, so the uploader might send data outside the
>firmware buffer. Perhaps:

We do validate the CRC for each microcode, it is unlikely the CRC
check can pass if the offset or length is not correct.  But you are
probably right that it will be safer to check the boundary and fail
quicker before we actually start the CRC check.  Will you come up with
a formal patch or you want us to deal with it?

>
>
> diff --git a/drivers/soc/fsl/qe/qe.c b/drivers/soc/fsl/qe/qe.c
> index 447146861c2c..c4e0bc452f03 100644
> --- a/drivers/soc/fsl/qe/qe.c
> +++ b/drivers/soc/fsl/qe/qe.c
> @@ -451,6 +451,7 @@ int qe_upload_firmware(const struct qe_firmware *firmware)
> size_t calc_size = sizeof(struct qe_firmware);
> size_t length;
> const struct qe_header *hdr;
> +   void *firmware_end;
>
> if (!firmware) {
> printk(KERN_ERR "qe-firmware: invalid pointer\n");
> @@ -491,19 +492,39 @@ int qe_upload_firmware(const struct qe_firmware 
> *firmware)
> calc_size += sizeof(__be32) *
> be32_to_cpu(firmware->microcode[i].count);
>
> -   /* Validate the length */
> +   /* Validate total length */
> if (length != calc_size + sizeof(__be32)) {
>   

Re: [PATCH] soc: fsl: Remove bogus packed attributes from qman.h

[Li Yang]

On Mon, Aug 31, 2020 at 8:57 PM Herbert Xu  wrote:
>
> On Tue, Sep 01, 2020 at 01:50:38AM +, Leo Li wrote:
> >
> > Sorry for the late response.  I missed this email previously.
> >
> > These structures are descriptors used by hardware, we cannot have _ANY_ 
> > padding from the compiler.  The compiled result might be the same with or 
> > without the __packed attribute for now, but I think keep it there probably 
> > is safer for dealing with unexpected alignment requirements from the 
> > compiler in the future.
> >
> > Having conflicting alignment requirements warning might means something is 
> > wrong with the structure in certain scenario.  I just tried a ARM64 build 
> > but didn't see the warnings.  Could you share the warning you got and the 
> > build setup?  Thanks.
>
> Just do a COMPILE_TEST build on x86-64:
>
> In file included from ../drivers/crypto/caam/qi.c:12:

Looks like the CAAM driver and dependent QBMAN driver doesn't support
COMPILE_TEST yet.  Are you trying to add the support for it?

I changed the Kconfig to enable the COMPILE_TEST anyway and updated my
toolchain to gcc-10 trying to duplicate the issue.  The issues can
only be reproduced with "W=1".

> ../include/soc/fsl/qman.h:259:1: warning: alignment 1 of ‘struct 
> qm_dqrr_entry’ is less than 8 [-Wpacked-not-aligned]
>  } __packed;
>  ^
> ../include/soc/fsl/qman.h:292:2: warning: alignment 1 of ‘struct ’ 
> is less than 8 [-Wpacked-not-aligned]
>   } __packed ern;
>   ^

I think this is a valid concern that if the parent structure doesn't
meet certain alignment requirements, the alignment for the
sub-structure cannot be guaranteed.  If we just remove the __packed
attribute from the parent structure, the compiler could try to add
padding in the parent structure to fulfill the alignment requirements
of the sub structure which is not good.  I think the following changes
are a better fix for the warnings:

diff --git a/include/soc/fsl/qman.h b/include/soc/fsl/qman.h
index cfe00e08e85b..9f484113cfda 100644
--- a/include/soc/fsl/qman.h
+++ b/include/soc/fsl/qman.h
@@ -256,7 +256,7 @@ struct qm_dqrr_entry {
__be32 context_b;
struct qm_fd fd;
u8 __reserved4[32];
-} __packed;
+} __packed __aligned(64);
 #define QM_DQRR_VERB_VBIT  0x80
 #define QM_DQRR_VERB_MASK  0x7f/* where the verb contains; */
 #define QM_DQRR_VERB_FRAME_DEQUEUE 0x60/* "this format" */
@@ -289,7 +289,7 @@ union qm_mr_entry {
__be32 tag;
struct qm_fd fd;
u8 __reserved1[32];
-   } __packed ern;
+   } __packed __aligned(64) ern;
struct {
u8 verb;
u8 fqs; /* Frame Queue Status */


Regards,
Leo

Re: [PATCH v1] soc: fsl: rcpm: Add ACPI support

[Li Yang]

On Wed, Sep 16, 2020 at 1:12 AM Ard Biesheuvel  wrote:
>
> On 9/16/20 3:32 AM, Ran Wang wrote:
> > Hi Ard,
> >
> > On Tuesday, September 15, 2020 7:10 PM, Ard Biesheuvel wrote:
> >> Subject: Re: [PATCH v1] soc: fsl: rcpm: Add ACPI support
> >>
> >> On 9/15/20 1:06 PM, kuldip dwivedi wrote:
> >>> Add ACPI support in fsl RCPM driver. This is required to support ACPI
> >>> S3 state. S3 is the ACPI sleep state that is known as "sleep" or
> >>> "suspend to RAM".
> >>> It essentially turns off most power of the system but keeps memory
> >>> powered.
> >>>
> >>> Signed-off-by: tanveer 
> >>> Signed-off-by: kuldip dwivedi 
> >>
> >> Why does the OS need to program this device? Can't this be done by
> >> firmware?
> >
> > This device is use to tell HW which IP (such as USB, SDHC, SATA, etc) 
> > should not be
> > clock gated during system enter low power state (to allow that IP work as a
> > wakeup source). And user does this configuration in device tree.
>
> The point of ACPI is *not* to describe a DT topology using a table
> format that is not suited for it. The point of ACPI is to describe a
> machine that is more abstracted from the hardware than is typically
> possible with DT, where the abstractions are implemented by AML code
> that is provided by the firmware, but executed in the context of the OS.
>
> So the idea is *not* finding the shortest possible path to get your
> existing DT driver code running on a system that boots via ACPI.
> Instead, you should carefully think about the abstract ACPI machine that
> you will expose to the OS, and hide everything else in firmware.
>
> In this particular case, it seems like your USB, SDHC and SATA device
> objects may need power state dependent AML methods that program this
> block directly.

The platform PM driver was created to support PM on systems without a
runtime PM firmware.   Even with PSCI firmware on later systems, there
is no standard interface to communicate the wakeup source information
directly from peripheral drivers to the PSCI firmware.  So we still
need this platform power management driver in kernel to deal with this
setup for non-ACPI scenarios.  From the code re-use perspective, I
think it is probably better to keep this generic implementation in
kernel instead of moving it to ACPI byte-code for each platform.

>
>
>
> > So implement
> > this RCPM driver to do it in kernel rather than firmware.
> >
> > Regards,
> > Ran
> >
> >>> ---
> >>>
> >>> Notes:
> >>>   1. Add ACPI match table
> >>>   2. NXP team members are added for confirming HID changes
> >>>   3. There is only one node in ACPI so no need to check for
> >>>  current device explicitly
> >>>   4. These changes are tested on LX2160A and LS1046A platforms
> >>>
> >>>drivers/soc/fsl/rcpm.c | 22 +++---
> >>>1 file changed, 19 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/drivers/soc/fsl/rcpm.c b/drivers/soc/fsl/rcpm.c index
> >>> a093dbe6d2cb..e75a436fb159 100644
> >>> --- a/drivers/soc/fsl/rcpm.c
> >>> +++ b/drivers/soc/fsl/rcpm.c
> >>> @@ -2,10 +2,12 @@
> >>>//
> >>>// rcpm.c - Freescale QorIQ RCPM driver
> >>>//
> >>> -// Copyright 2019 NXP
> >>> +// Copyright 2019-2020 NXP
> >>> +// Copyright 2020 Puresoftware Ltd.
> >>>//
> >>>// Author: Ran Wang 
> >>>
> >>> +#include 
> >>>#include 
> >>>#include 
> >>>#include 
> >>> @@ -57,8 +59,13 @@ static int rcpm_pm_prepare(struct device *dev)
> >>> rcpm->wakeup_cells + 1);
> >>>
> >>> /*  Wakeup source should refer to current rcpm device */
> >>> -   if (ret || (np->phandle != value[0]))
> >>> -   continue;
> >>> +   if (is_acpi_node(dev->fwnode)) {
> >>> +   if (ret)
> >>> +   continue;
> >>> +   } else {
> >>> +   if (ret || (np->phandle != value[0]))
> >>> +   continue;
> >>> +   }
> >>>
> >>> /* Property "#fsl,rcpm-wakeup-cells" of rcpm node defines the
> >>>  * number of IPPDEXPCR register cells, and "fsl,rcpm-wakeup"
> >>> @@ -139,10 +146,19 @@ static const struct of_device_id rcpm_of_match[]
> >> = {
> >>>};
> >>>MODULE_DEVICE_TABLE(of, rcpm_of_match);
> >>>
> >>> +#ifdef CONFIG_ACPI
> >>> +static const struct acpi_device_id rcpm_acpi_match[] = {
> >>> +   { "NXP0015", },
> >>> +   { }
> >>> +};
> >>> +MODULE_DEVICE_TABLE(acpi, rcpm_acpi_match); #endif
> >>> +
> >>>static struct platform_driver rcpm_driver = {
> >>> .driver = {
> >>> .name = "rcpm",
> >>> .of_match_table = rcpm_of_match,
> >>> +   .acpi_match_table = ACPI_PTR(rcpm_acpi_match),
> >>> .pm = _pm_ops,
> >>> },
> >>> .probe = rcpm_probe,
> >>>
> >
>

Re: [PATCH -next] soc/qman: convert to use be32_add_cpu()

[Li Yang]

On Sun, Sep 13, 2020 at 10:56 PM Liu Shixin  wrote:
>
> Signed-off-by: Liu Shixin 
> drivers/soc/fsl/qbman/qman_test_api.c---

The patch seems to be messed up here.

I have fixed that, and applied for next.  Thanks.

>  drivers/soc/fsl/qbman/qman_test_api.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/soc/fsl/qbman/qman_test_api.c 
> b/drivers/soc/fsl/qbman/qman_test_api.c
> index 2895d062cf51..7066b2f1467c 100644
> --- a/drivers/soc/fsl/qbman/qman_test_api.c
> +++ b/drivers/soc/fsl/qbman/qman_test_api.c
> @@ -86,7 +86,7 @@ static void fd_inc(struct qm_fd *fd)
> len--;
> qm_fd_set_param(fd, fmt, off, len);
>
> -   fd->cmd = cpu_to_be32(be32_to_cpu(fd->cmd) + 1);
> +   be32_add_cpu(>cmd, 1);
>  }
>
>  /* The only part of the 'fd' we can't memcmp() is the ppid */
> --
> 2.25.1
>

Re: [PATCH] soc: fsl: dpio: remove set but not used 'addr_cena'

[Li Yang]

On Sun, Sep 20, 2020 at 3:20 PM Krzysztof Kozlowski  wrote:
>
> On Thu, 10 Sep 2020 at 16:57, Jason Yan  wrote:
> >
> > This addresses the following gcc warning with "make W=1":
> >
> > drivers/soc/fsl/dpio/qbman-portal.c: In function
> > ‘qbman_swp_enqueue_multiple_direct’:
> > drivers/soc/fsl/dpio/qbman-portal.c:650:11: warning: variable
> > ‘addr_cena’ set but not used [-Wunused-but-set-variable]
> >   650 |  uint64_t addr_cena;
> >   |   ^
> >
> > Reported-by: Hulk Robot 
> > Signed-off-by: Jason Yan 
>
> This was already reported:
> Reported-by: kernel test robot 
> https://lkml.org/lkml/2020/6/12/290
>
> Reviewed-by: Krzysztof Kozlowski 

Applied for next.  Thanks.

>
> Best regards,
> Krzysztof
>
> > ---
> >  drivers/soc/fsl/dpio/qbman-portal.c | 2 --
> >  1 file changed, 2 deletions(-)
> >
> > diff --git a/drivers/soc/fsl/dpio/qbman-portal.c 
> > b/drivers/soc/fsl/dpio/qbman-portal.c
> > index 0ab85bfb116f..659b4a570d5b 100644
> > --- a/drivers/soc/fsl/dpio/qbman-portal.c
> > +++ b/drivers/soc/fsl/dpio/qbman-portal.c
> > @@ -647,7 +647,6 @@ int qbman_swp_enqueue_multiple_direct(struct qbman_swp 
> > *s,
> > const uint32_t *cl = (uint32_t *)d;
> > uint32_t eqcr_ci, eqcr_pi, half_mask, full_mask;
> > int i, num_enqueued = 0;
> > -   uint64_t addr_cena;
> >
> > spin_lock(>access_spinlock);
> > half_mask = (s->eqcr.pi_ci_mask>>1);
> > @@ -701,7 +700,6 @@ int qbman_swp_enqueue_multiple_direct(struct qbman_swp 
> > *s,
> >
> > /* Flush all the cacheline without load/store in between */
> > eqcr_pi = s->eqcr.pi;
> > -   addr_cena = (size_t)s->addr_cena;
> > for (i = 0; i < num_enqueued; i++)
> > eqcr_pi++;
> > s->eqcr.pi = eqcr_pi & full_mask;
> > --
> > 2.25.4
> >

Re: [PATCH -next] soc: fsl: qe: Remove unnessesary check in ucc_set_tdm_rxtx_clk

[Li Yang]

On Tue, Aug 4, 2020 at 9:04 AM Wang Hai  wrote:
>
> Fix smatch warning:
>
> drivers/soc/fsl/qe/ucc.c:526
>  ucc_set_tdm_rxtx_clk() warn: unsigned 'tdm_num' is never less than zero.
>
> 'tdm_num' is u32 type, never less than zero.
>
> Signed-off-by: Wang Hai 

Applied for next.  Thanks.

> ---
>  drivers/soc/fsl/qe/ucc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/soc/fsl/qe/ucc.c b/drivers/soc/fsl/qe/ucc.c
> index cac0fb7693a0..21dbcd787cd5 100644
> --- a/drivers/soc/fsl/qe/ucc.c
> +++ b/drivers/soc/fsl/qe/ucc.c
> @@ -523,7 +523,7 @@ int ucc_set_tdm_rxtx_clk(u32 tdm_num, enum qe_clock clock,
>
> qe_mux_reg = _immr->qmx;
>
> -   if (tdm_num > 7 || tdm_num < 0)
> +   if (tdm_num > 7)
> return -EINVAL;
>
> /* The communications direction must be RX or TX */
> --
> 2.17.1
>

Re: [PATCH] soc: fsl: qbman: Fix return value on success

[Li Yang]

On Sun, Sep 20, 2020 at 3:27 PM Krzysztof Kozlowski  wrote:
>
> On error the function was meant to return -ERRNO.  This also fixes
> compile warning:
>
>   drivers/soc/fsl/qbman/bman.c:640:6: warning: variable 'err' set but not 
> used [-Wunused-but-set-variable]
>
> Fixes: 0505d00c8dba ("soc/fsl/qbman: Cleanup buffer pools if BMan was 
> initialized prior to bootup")
> Signed-off-by: Krzysztof Kozlowski 

Applied for next.  Thanks.

> ---
>  drivers/soc/fsl/qbman/bman.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/soc/fsl/qbman/bman.c b/drivers/soc/fsl/qbman/bman.c
> index f4fb527d8301..c5dd026fe889 100644
> --- a/drivers/soc/fsl/qbman/bman.c
> +++ b/drivers/soc/fsl/qbman/bman.c
> @@ -660,7 +660,7 @@ int bm_shutdown_pool(u32 bpid)
> }
>  done:
> put_affine_portal();
> -   return 0;
> +   return err;
>  }
>
>  struct gen_pool *bm_bpalloc;
> --
> 2.17.1
>

Re: [v3 2/2] dts: ppc: t1024rdb: remove interrupts property

[Li Yang]

On Tue, May 26, 2020 at 10:52 PM Biwen Li  wrote:
>
> From: Biwen Li 
>
> Since the interrupt pin for RTC DS1339 is not connected
> to the CPU on T1024RDB, remove the interrupt property
> from the device tree.
>
> This also fix the following warning for hwclock.util-linux:
> $ hwclock.util-linux
> hwclock.util-linux: select() to /dev/rtc0
> to wait for clock tick timed out
>
> Signed-off-by: Biwen Li 

Acked-by: Li Yang 

> ---
>  arch/powerpc/boot/dts/fsl/t1024rdb.dts | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/arch/powerpc/boot/dts/fsl/t1024rdb.dts 
> b/arch/powerpc/boot/dts/fsl/t1024rdb.dts
> index 645caff98ed1..605ceec66af3 100644
> --- a/arch/powerpc/boot/dts/fsl/t1024rdb.dts
> +++ b/arch/powerpc/boot/dts/fsl/t1024rdb.dts
> @@ -161,7 +161,6 @@
> rtc@68 {
> compatible = "dallas,ds1339";
> reg = <0x68>;
> -   interrupts = <0x1 0x1 0 0>;
> };
> };
>
> --
> 2.17.1
>

Re: [v3 1/2] dts: ppc: t4240rdb: remove interrupts property

[Li Yang]

On Tue, May 26, 2020 at 10:49 PM Biwen Li  wrote:
>
> From: Biwen Li 
>
> Since the interrupt pin for RTC DS1374 is not connected
> to the CPU on T4240RDB, remove the interrupt property
> from the device tree.
>
> This also fix the following warning for hwclock.util-linux:
> $ hwclock.util-linux
> hwclock.util-linux: select() to /dev/rtc0
> to wait for clock tick timed out
>
> Signed-off-by: Biwen Li 

Acked-by: Li Yang 

> ---
>  arch/powerpc/boot/dts/fsl/t4240rdb.dts | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/arch/powerpc/boot/dts/fsl/t4240rdb.dts 
> b/arch/powerpc/boot/dts/fsl/t4240rdb.dts
> index a56a705d41f7..145896f2eef6 100644
> --- a/arch/powerpc/boot/dts/fsl/t4240rdb.dts
> +++ b/arch/powerpc/boot/dts/fsl/t4240rdb.dts
> @@ -144,7 +144,6 @@
> rtc@68 {
> compatible = "dallas,ds1374";
> reg = <0x68>;
> -   interrupts = <0x1 0x1 0 0>;
> };
> };
>
> --
> 2.17.1
>

Re: [PATCH] soc: fsl: qe: Replace one-element array and use struct_size() helper

[Li Yang]

On Sun, May 24, 2020 at 9:49 PM Qiang Zhao  wrote:
>
> On Wed, May 23, 2020 at 5:22 PM Li Yang 
> > -Original Message-
> > From: Li Yang 
> > Sent: 2020年5月23日 5:22
> > To: Kees Cook 
> > Cc: Gustavo A. R. Silva ; Qiang Zhao
> > ; linuxppc-dev ;
> > moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
> > ; lkml ;
> > Gustavo A. R. Silva 
> > Subject: Re: [PATCH] soc: fsl: qe: Replace one-element array and use
> > struct_size() helper
> >
> > On Wed, May 20, 2020 at 10:24 PM Kees Cook 
> > wrote:
> > >
> > > On Wed, May 20, 2020 at 06:52:21PM -0500, Li Yang wrote:
> > > > On Mon, May 18, 2020 at 5:57 PM Kees Cook 
> > wrote:
> > > > > Hm, looking at this code, I see a few other things that need to be
> > > > > fixed:
> > > > >
> > > > > 1) drivers/tty/serial/ucc_uart.c does not do a be32_to_cpu() 
> > > > > conversion
> > > > >on the length test (understandably, a little-endian system has 
> > > > > never
> > run
> > > > >this code since it's ppc specific), but it's still wrong:
> > > > >
> > > > > if (firmware->header.length != fw->size) {
> > > > >
> > > > >compare to the firmware loader:
> > > > >
> > > > > length = be32_to_cpu(hdr->length);
> > > > >
> > > > > 2) drivers/soc/fsl/qe/qe.c does not perform bounds checking on the
> > > > >per-microcode offsets, so the uploader might send data outside the
> > > > >firmware buffer. Perhaps:
> > > >
> > > > We do validate the CRC for each microcode, it is unlikely the CRC
> > > > check can pass if the offset or length is not correct.  But you are
> > > > probably right that it will be safer to check the boundary and fail
> > >
> > > Right, but a malicious firmware file could still match CRC but trick
> > > the kernel code.
> > >
> > > > quicker before we actually start the CRC check.  Will you come up
> > > > with a formal patch or you want us to deal with it?
> > >
> > > It sounds like Gustavo will be sending one, though I don't think
> > > either of us have the hardware to test it with, so if you could do
> > > that part, that would be great! :)
> >
> > That will be great.  I think Zhao Qiang can help with the testing part.
> >
>
> Now the firmware are loaded in uboot, and kernel will do nothing for it.
> So testing on it maybe need some extra codes both in driver and dts.
> In the meanwhile, I am so busy on some high priority work that maybe test work
> could not be done in time.
> Once I am free, I will do it.

Thanks.  You are right that most of the QE drivers doesn't support
requesting firmware in kernel except the ucc_uart.  So it probably can
be tested with that driver without requiring code change.

>
> Best Regards
> Qiang Zhao

Re: [v2 2/2] dts: ppc: t1024rdb: remove interrupts property

[Li Yang]

On Wed, May 20, 2020 at 4:21 AM Biwen Li  wrote:
>
> From: Biwen Li 
>
> This removes interrupts property to drop warning as follows:
> - $ hwclock.util-linux
>   hwclock.util-linux: select() to /dev/rtc0
>   to wait for clock tick timed out
>
> My case:
> - RTC ds1339s INT pin isn't connected to cpus INT pin on T1024RDB,
>   then the RTC cannot inform cpu about alarm interrupt
>
> How to fix it?
> - remove IRQ line

This style is not the recommended style for commit message.  Please
see my comment for the other patch.

>
> Signed-off-by: Biwen Li 
> ---
>  arch/powerpc/boot/dts/fsl/t1024rdb.dts | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/arch/powerpc/boot/dts/fsl/t1024rdb.dts 
> b/arch/powerpc/boot/dts/fsl/t1024rdb.dts
> index 645caff98ed1..605ceec66af3 100644
> --- a/arch/powerpc/boot/dts/fsl/t1024rdb.dts
> +++ b/arch/powerpc/boot/dts/fsl/t1024rdb.dts
> @@ -161,7 +161,6 @@
> rtc@68 {
> compatible = "dallas,ds1339";
> reg = <0x68>;
> -   interrupts = <0x1 0x1 0 0>;
> };
> };
>
> --
> 2.17.1
>

Re: [v2 1/2] dts: ppc: t4240rdb: remove interrupts property

[Li Yang]

On Wed, May 20, 2020 at 4:21 AM Biwen Li  wrote:
>
> From: Biwen Li 
>
> This removes interrupts property to drop warning as follows:
> - $ hwclock.util-linux
>   hwclock.util-linux: select() to /dev/rtc0
>   to wait for clock tick timed out
>
> My case:
> - RTC ds1374's INT pin is connected to VCC on T4240RDB,
>   then the RTC cannot inform cpu about the alarm interrupt

The commit message need a little bit improvement.  Something like:

Since the interrupt pin for RTC DS1374 is not connected to the CPU on
T4240RDB, remove
the interrupt property from the device tree.

This also fix the following warning for hwclock.util-linux:
  $ hwclock.util-linux
   hwclock.util-linux: select() to /dev/rtc0
   to wait for clock tick timed out

>
> Signed-off-by: Biwen Li 
> ---
>  arch/powerpc/boot/dts/fsl/t4240rdb.dts | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/arch/powerpc/boot/dts/fsl/t4240rdb.dts 
> b/arch/powerpc/boot/dts/fsl/t4240rdb.dts
> index a56a705d41f7..145896f2eef6 100644
> --- a/arch/powerpc/boot/dts/fsl/t4240rdb.dts
> +++ b/arch/powerpc/boot/dts/fsl/t4240rdb.dts
> @@ -144,7 +144,6 @@
> rtc@68 {
> compatible = "dallas,ds1374";
> reg = <0x68>;
> -   interrupts = <0x1 0x1 0 0>;
> };
> };
>
> --
> 2.17.1
>

Re: [PATCH] soc: fsl: qe: clean up an indentation issue

[Li Yang]

On Fri, Mar 27, 2020 at 11:15 AM Colin King  wrote:
>
> From: Colin Ian King 
>
> There is a statement that not indented correctly, remove the
> extraneous space.
>
> Signed-off-by: Colin Ian King 

Applied for next.  Thanks.

> ---
>  drivers/soc/fsl/qe/ucc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/soc/fsl/qe/ucc.c b/drivers/soc/fsl/qe/ucc.c
> index d6c93970df4d..cac0fb7693a0 100644
> --- a/drivers/soc/fsl/qe/ucc.c
> +++ b/drivers/soc/fsl/qe/ucc.c
> @@ -519,7 +519,7 @@ int ucc_set_tdm_rxtx_clk(u32 tdm_num, enum qe_clock clock,
> int clock_bits;
> u32 shift;
> struct qe_mux __iomem *qe_mux_reg;
> -__be32 __iomem *cmxs1cr;
> +   __be32 __iomem *cmxs1cr;
>
> qe_mux_reg = _immr->qmx;
>
> --
> 2.25.1
>

Re: [PATCH -next] soc: fsl: dpio: Remove unused inline function qbman_write_eqcr_am_rt_register

[Li Yang]

On Fri, May 8, 2020 at 9:13 AM YueHaibing  wrote:
>
> There's no callers in-tree anymore since commit
> 3b2abda7d28c ("soc: fsl: dpio: Replace QMAN array mode with ring mode 
> enqueue")
>
> Signed-off-by: YueHaibing 

Applied to next.  Thanks.

Regards,
Leo
> ---
>  drivers/soc/fsl/dpio/qbman-portal.c | 12 
>  1 file changed, 12 deletions(-)
>
> diff --git a/drivers/soc/fsl/dpio/qbman-portal.c 
> b/drivers/soc/fsl/dpio/qbman-portal.c
> index 804b8ba9bf5c..e2e9fbb58a72 100644
> --- a/drivers/soc/fsl/dpio/qbman-portal.c
> +++ b/drivers/soc/fsl/dpio/qbman-portal.c
> @@ -572,18 +572,6 @@ void qbman_eq_desc_set_qd(struct qbman_eq_desc *d, u32 
> qdid,
>  #define EQAR_VB(eqar)  ((eqar) & 0x80)
>  #define EQAR_SUCCESS(eqar) ((eqar) & 0x100)
>
> -static inline void qbman_write_eqcr_am_rt_register(struct qbman_swp *p,
> -  u8 idx)
> -{
> -   if (idx < 16)
> -   qbman_write_register(p, QBMAN_CINH_SWP_EQCR_AM_RT + idx * 4,
> -QMAN_RT_MODE);
> -   else
> -   qbman_write_register(p, QBMAN_CINH_SWP_EQCR_AM_RT2 +
> -(idx - 16) * 4,
> -QMAN_RT_MODE);
> -}
> -
>  #define QB_RT_BIT ((u32)0x100)
>  /**
>   * qbman_swp_enqueue_direct() - Issue an enqueue command
> --
> 2.17.1
>
>

Re: [PATCH] soc: fsl: qe: Replace one-element array and use struct_size() helper

[Li Yang]

On Wed, May 20, 2020 at 10:24 PM Kees Cook  wrote:
>
> On Wed, May 20, 2020 at 06:52:21PM -0500, Li Yang wrote:
> > On Mon, May 18, 2020 at 5:57 PM Kees Cook  wrote:
> > > Hm, looking at this code, I see a few other things that need to be
> > > fixed:
> > >
> > > 1) drivers/tty/serial/ucc_uart.c does not do a be32_to_cpu() conversion
> > >on the length test (understandably, a little-endian system has never 
> > > run
> > >this code since it's ppc specific), but it's still wrong:
> > >
> > > if (firmware->header.length != fw->size) {
> > >
> > >compare to the firmware loader:
> > >
> > > length = be32_to_cpu(hdr->length);
> > >
> > > 2) drivers/soc/fsl/qe/qe.c does not perform bounds checking on the
> > >per-microcode offsets, so the uploader might send data outside the
> > >firmware buffer. Perhaps:
> >
> > We do validate the CRC for each microcode, it is unlikely the CRC
> > check can pass if the offset or length is not correct.  But you are
> > probably right that it will be safer to check the boundary and fail
>
> Right, but a malicious firmware file could still match CRC but trick the
> kernel code.
>
> > quicker before we actually start the CRC check.  Will you come up with
> > a formal patch or you want us to deal with it?
>
> It sounds like Gustavo will be sending one, though I don't think either
> of us have the hardware to test it with, so if you could do that part,
> that would be great! :)

That will be great.  I think Zhao Qiang can help with the testing part.

Regards,
Leo

Re: [PATCH] treewide: Replace zero-length array with flexible-array

[Li Yang]

On Thu, May 7, 2020 at 1:49 PM Gustavo A. R. Silva
 wrote:
>
> The current codebase makes use of the zero-length array language
> extension to the C90 standard, but the preferred mechanism to declare
> variable-length types such as these ones is a flexible array member[1][2],
> introduced in C99:
>
> struct foo {
> int stuff;
> struct boo array[];
> };
>
> By making use of the mechanism above, we will get a compiler warning
> in case the flexible array does not occur last in the structure, which
> will help us prevent some kind of undefined behavior bugs from being
> inadvertently introduced[3] to the codebase from now on.
>
> Also, notice that, dynamic memory allocations won't be affected by
> this change:
>
> "Flexible array members have incomplete type, and so the sizeof operator
> may not be applied. As a quirk of the original implementation of
> zero-length arrays, sizeof evaluates to zero."[1]
>
> sizeof(flexible-array-member) triggers a warning because flexible array
> members have incomplete type[1]. There are some instances of code in
> which the sizeof operator is being incorrectly/erroneously applied to
> zero-length arrays and the result is zero. Such instances may be hiding
> some bugs. So, this work (flexible-array member conversions) will also
> help to get completely rid of those sorts of issues.
>
> This issue was found with the help of Coccinelle.
>
> [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
> [2] https://github.com/KSPP/linux/issues/21
> [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
>
> Signed-off-by: Gustavo A. R. Silva 
> ---
>  include/linux/fsl/bestcomm/bestcomm.h |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied for next.  Thanks.

Regards,
Leo

>
> diff --git a/include/linux/fsl/bestcomm/bestcomm.h 
> b/include/linux/fsl/bestcomm/bestcomm.h
> index a0e2e6b19b57..154e541ce57e 100644
> --- a/include/linux/fsl/bestcomm/bestcomm.h
> +++ b/include/linux/fsl/bestcomm/bestcomm.h
> @@ -27,7 +27,7 @@
>   */
>  struct bcom_bd {
> u32 status;
> -   u32 data[0];/* variable payload size */
> +   u32 data[]; /* variable payload size */
>  };
>
>  /*  
> */
>

Re: [PATCH] usb: gadget: fsl: Fix a wrong judgment in fsl_udc_probe()

[Li Yang]

On Tue, Apr 14, 2020 at 4:13 AM Tang Bin  wrote:
>
> Hi
>
> On 2020/4/14 16:30, Dan Carpenter wrote:
> > On Fri, Apr 10, 2020 at 04:05:06PM +0800, Tang Bin wrote:
> >>>
> >>>> Thus it must be fixed.
> >>> Wording alternative:
> >>> Thus adjust the error detection and corresponding exception handling.
> >> Got it.
> > Wow...
> >
> > No, don't listen to Markus when it comes to writing commit messages.
> > You couldn't find worse advice anywhere.  :P
>
> I'm actively waiting for a reply from the maintainer. Thank you very much.

Sorry for the late response.

Acked-by: Li Yang 

Regards,
Leo

Re: [PATCH] soc: fsl: qe: Replace one-element array and use struct_size() helper

[Li Yang]

On Mon, May 18, 2020 at 5:16 PM Gustavo A. R. Silva
 wrote:
>
> The current codebase makes use of one-element arrays in the following
> form:
>
> struct something {
> int length;
> u8 data[1];
> };
>
> struct something *instance;
>
> instance = kmalloc(sizeof(*instance) + size, GFP_KERNEL);
> instance->length = size;
> memcpy(instance->data, source, size);
>
> but the preferred mechanism to declare variable-length types such as
> these ones is a flexible array member[1][2], introduced in C99:
>
> struct foo {
> int stuff;
> struct boo array[];
> };
>
> By making use of the mechanism above, we will get a compiler warning
> in case the flexible array does not occur last in the structure, which
> will help us prevent some kind of undefined behavior bugs from being
> inadvertently introduced[3] to the codebase from now on. So, replace
> the one-element array with a flexible-array member.
>
> Also, make use of the new struct_size() helper to properly calculate the
> size of struct qe_firmware.
>
> This issue was found with the help of Coccinelle and, audited and fixed
> _manually_.
>
> [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
> [2] https://github.com/KSPP/linux/issues/21
> [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
>
> Signed-off-by: Gustavo A. R. Silva 
> ---
>  drivers/soc/fsl/qe/qe.c | 4 ++--
>  include/soc/fsl/qe/qe.h | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)

Applied for next.  Thanks.

Regards,
Leo

Re: [PATCH -next] soc: fsl: qbman: Remove unused inline function qm_eqcr_get_ci_stashing

[Li Yang]

On Fri, May 8, 2020 at 9:11 AM YueHaibing  wrote:
>
> There's no callers in-tree anymore.
>
> Signed-off-by: YueHaibing 

Applied for next.  Thanks.

Regards,
Leo
> ---
>  drivers/soc/fsl/qbman/qman.c | 5 -
>  1 file changed, 5 deletions(-)
>
> diff --git a/drivers/soc/fsl/qbman/qman.c b/drivers/soc/fsl/qbman/qman.c
> index 1e164e03410a..9888a7061873 100644
> --- a/drivers/soc/fsl/qbman/qman.c
> +++ b/drivers/soc/fsl/qbman/qman.c
> @@ -449,11 +449,6 @@ static inline int qm_eqcr_init(struct qm_portal *portal,
> return 0;
>  }
>
> -static inline unsigned int qm_eqcr_get_ci_stashing(struct qm_portal *portal)
> -{
> -   return (qm_in(portal, QM_REG_CFG) >> 28) & 0x7;
> -}
> -
>  static inline void qm_eqcr_finish(struct qm_portal *portal)
>  {
> struct qm_eqcr *eqcr = >eqcr;
> --
> 2.17.1
>
>

Re: [PATCH] net/ethernet/freescale: Fix incorrect IS_ERR_VALUE macro usages

[Li Yang]

On Tue, Nov 24, 2020 at 8:00 PM liwei (GF)  wrote:
>
> Hi Yang,
>
> On 2020/11/25 6:13, Li Yang wrote:
> > On Tue, Nov 24, 2020 at 3:44 PM Li Yang  wrote:
> >>
> >> On Tue, Nov 24, 2020 at 12:24 AM Wei Li  wrote:
> >>>
> >>> IS_ERR_VALUE macro should be used only with unsigned long type.
> >>> Especially it works incorrectly with unsigned shorter types on
> >>> 64bit machines.
> >>
> >> This is truly a problem for the driver to run on 64-bit architectures.
> >> But from an earlier discussion
> >> https://patchwork.kernel.org/project/linux-kbuild/patch/1464384685-347275-1-git-send-email-a...@arndb.de/,
> >> the preferred solution would be removing the IS_ERR_VALUE() usage or
> >> make the values to be unsigned long.
> >>
> >> It looks like we are having a bigger problem with the 64-bit support
> >> for the driver that the offset variables can also be real pointers
> >> which cannot be held with 32-bit data types(when uf_info->bd_mem_part
> >> == MEM_PART_SYSTEM).  So actually we have to change these offsets to
> >> unsigned long, otherwise we are having more serious issues on 64-bit
> >> systems.  Are you willing to make such changes or you want us to deal
> >> with it?
> >
> > Well, it looks like this hardware block was never integrated on a
> > 64-bit SoC and will very likely to keep so.  So probably we can keep
> > the driver 32-bit only.  It is currently limited to PPC32 in Kconfig,
> > how did you build it for 64-bit?
> >
> >>
>
> Thank you for providing the earlier discussion archive. In fact, this
> issue is detected by our static analysis tool.

Thanks for the effort, but this probably is a false positive for the
static analysis tool as the 64-bit case is not buildable.

>
> From my view, there is no harm to fix these potential misuses. But if you
> really have decided to keep the driver 32-bit only, please just ingore this 
> patch.

It is not an easy task to add proper 64-bit support, so probably we
just keep it 32-bit only for now.  Thanks for the patch anyway.

Regards,
Leo

>
> Thanks,
> Wei
>
> >>>
> >>> Fixes: 4c35630ccda5 ("[POWERPC] Change rheap functions to use ulongs 
> >>> instead of pointers")
> >>> Signed-off-by: Wei Li 
> >>> ---
> >>>  drivers/net/ethernet/freescale/ucc_geth.c | 30 +++
> >>>  1 file changed, 15 insertions(+), 15 deletions(-)
> >>>
> >>> diff --git a/drivers/net/ethernet/freescale/ucc_geth.c 
> >>> b/drivers/net/ethernet/freescale/ucc_geth.c
> >>> index 714b501be7d0..8656d9be256a 100644
> >>> --- a/drivers/net/ethernet/freescale/ucc_geth.c
> >>> +++ b/drivers/net/ethernet/freescale/ucc_geth.c
> >>> @@ -286,7 +286,7 @@ static int fill_init_enet_entries(struct 
> >>> ucc_geth_private *ugeth,
> >>> else {
> >>> init_enet_offset =
> >>> qe_muram_alloc(thread_size, thread_alignment);
> >>> -   if (IS_ERR_VALUE(init_enet_offset)) {
> >>> +   if (IS_ERR_VALUE((unsigned 
> >>> long)(int)init_enet_offset)) {
> >>> if (netif_msg_ifup(ugeth))
> >>> pr_err("Can not allocate DPRAM 
> >>> memory\n");
> >>> qe_put_snum((u8) snum);
> >>> @@ -2223,7 +2223,7 @@ static int ucc_geth_alloc_tx(struct 
> >>> ucc_geth_private *ugeth)
> >>> ugeth->tx_bd_ring_offset[j] =
> >>> qe_muram_alloc(length,
> >>>UCC_GETH_TX_BD_RING_ALIGNMENT);
> >>> -   if (!IS_ERR_VALUE(ugeth->tx_bd_ring_offset[j]))
> >>> +   if (!IS_ERR_VALUE((unsigned 
> >>> long)(int)ugeth->tx_bd_ring_offset[j]))
> >>> ugeth->p_tx_bd_ring[j] =
> >>> (u8 __iomem *) qe_muram_addr(ugeth->
> >>>  
> >>> tx_bd_ring_offset[j]);
> >>> @@ -2300,7 +2300,7 @@ static int ucc_geth_alloc_rx(struct 
> >>> ucc_geth_private *ugeth)
> >>> ugeth->rx_bd_ring_offset[j] =
> >>> qe_muram_alloc(le

Re: [PATCH 02/20] ethernet: ucc_geth: fix definition and size of ucc_geth_tx_global_pram

[Li Yang]

On Sat, Dec 5, 2020 at 1:21 PM Rasmus Villemoes
 wrote:
>
> Table 8-53 in the QUICC Engine Reference manual shows definitions of
> fields up to a size of 192 bytes, not just 128. But in table 8-111,
> one does find the text
>
>   Base Address of the Global Transmitter Parameter RAM Page. [...]
>   The user needs to allocate 128 bytes for this page. The address must
>   be aligned to the page size.
>
> I've checked both rev. 7 (11/2015) and rev. 9 (05/2018) of the manual;
> they both have this inconsistency (and the table numbers are the
> same).

This does seem to be an inconsistency.  I will try to see if I can
find someone who is familiar with this as this is really an old IP.

Figure 8-61 does mention that size = 128 byte + 64 byte if But
this part is not clear also.  Not sure if the size of the parameter
RAM is really conditional.

>
> Adding a bit of debug printing, on my board the struct
> ucc_geth_tx_global_pram is allocated at offset 0x880, while
> the (opaque) ucc_geth_thread_data_tx gets allocated immediately
> afterwards, at 0x900. So whatever the engine writes into the thread
> data overlaps with the tail of the global tx pram (and devmem says
> that something does get written during a simple ping).

The overlapping does seem to be a problem.  Maybe these global
parameters are not sampled at runtime or the parameter RAM is really
only using 128byte depending on the operation mode.

Are you getting useful information by reading from the additional 64
bytes, or getting changed behavior for setting these bytes after your
changes?

>
> I haven't observed any failure that could be attributed to this, but
> it seems to be the kind of thing that would be extremely hard to
> debug. So extend the struct definition so that we do allocate 192
> bytes.
>
> Signed-off-by: Rasmus Villemoes 
> ---
>  drivers/net/ethernet/freescale/ucc_geth.h | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/freescale/ucc_geth.h 
> b/drivers/net/ethernet/freescale/ucc_geth.h
> index 3fe903972195..c80bed2c995c 100644
> --- a/drivers/net/ethernet/freescale/ucc_geth.h
> +++ b/drivers/net/ethernet/freescale/ucc_geth.h
> @@ -575,7 +575,14 @@ struct ucc_geth_tx_global_pram {
> u32 vtagtable[0x8]; /* 8 4-byte VLAN tags */
> u32 tqptr;  /* a base pointer to the Tx Queues Memory
>Region */
> -   u8 res2[0x80 - 0x74];
> +   u8 res2[0x78 - 0x74];
> +   u64 snums_en;
> +   u32 l2l3baseptr;/* top byte consists of a few other bit 
> fields */
> +
> +   u16 mtu[8];
> +   u8 res3[0xa8 - 0x94];
> +   u32 wrrtablebase;   /* top byte is reserved */
> +   u8 res4[0xc0 - 0xac];
>  } __packed;
>
>  /* structure representing Extended Filtering Global Parameters in PRAM */
> --
> 2.23.0
>

Re: [PATCH] net/ethernet/freescale: Fix incorrect IS_ERR_VALUE macro usages

[Li Yang]

On Tue, Nov 24, 2020 at 12:24 AM Wei Li  wrote:
>
> IS_ERR_VALUE macro should be used only with unsigned long type.
> Especially it works incorrectly with unsigned shorter types on
> 64bit machines.

This is truly a problem for the driver to run on 64-bit architectures.
But from an earlier discussion
https://patchwork.kernel.org/project/linux-kbuild/patch/1464384685-347275-1-git-send-email-a...@arndb.de/,
the preferred solution would be removing the IS_ERR_VALUE() usage or
make the values to be unsigned long.

It looks like we are having a bigger problem with the 64-bit support
for the driver that the offset variables can also be real pointers
which cannot be held with 32-bit data types(when uf_info->bd_mem_part
== MEM_PART_SYSTEM).  So actually we have to change these offsets to
unsigned long, otherwise we are having more serious issues on 64-bit
systems.  Are you willing to make such changes or you want us to deal
with it?

Regards,
Leo
>
> Fixes: 4c35630ccda5 ("[POWERPC] Change rheap functions to use ulongs instead 
> of pointers")
> Signed-off-by: Wei Li 
> ---
>  drivers/net/ethernet/freescale/ucc_geth.c | 30 +++
>  1 file changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/ucc_geth.c 
> b/drivers/net/ethernet/freescale/ucc_geth.c
> index 714b501be7d0..8656d9be256a 100644
> --- a/drivers/net/ethernet/freescale/ucc_geth.c
> +++ b/drivers/net/ethernet/freescale/ucc_geth.c
> @@ -286,7 +286,7 @@ static int fill_init_enet_entries(struct ucc_geth_private 
> *ugeth,
> else {
> init_enet_offset =
> qe_muram_alloc(thread_size, thread_alignment);
> -   if (IS_ERR_VALUE(init_enet_offset)) {
> +   if (IS_ERR_VALUE((unsigned 
> long)(int)init_enet_offset)) {
> if (netif_msg_ifup(ugeth))
> pr_err("Can not allocate DPRAM 
> memory\n");
> qe_put_snum((u8) snum);
> @@ -2223,7 +2223,7 @@ static int ucc_geth_alloc_tx(struct ucc_geth_private 
> *ugeth)
> ugeth->tx_bd_ring_offset[j] =
> qe_muram_alloc(length,
>UCC_GETH_TX_BD_RING_ALIGNMENT);
> -   if (!IS_ERR_VALUE(ugeth->tx_bd_ring_offset[j]))
> +   if (!IS_ERR_VALUE((unsigned 
> long)(int)ugeth->tx_bd_ring_offset[j]))
> ugeth->p_tx_bd_ring[j] =
> (u8 __iomem *) qe_muram_addr(ugeth->
>  
> tx_bd_ring_offset[j]);
> @@ -2300,7 +2300,7 @@ static int ucc_geth_alloc_rx(struct ucc_geth_private 
> *ugeth)
> ugeth->rx_bd_ring_offset[j] =
> qe_muram_alloc(length,
>UCC_GETH_RX_BD_RING_ALIGNMENT);
> -   if (!IS_ERR_VALUE(ugeth->rx_bd_ring_offset[j]))
> +   if (!IS_ERR_VALUE((unsigned 
> long)(int)ugeth->rx_bd_ring_offset[j]))
> ugeth->p_rx_bd_ring[j] =
> (u8 __iomem *) qe_muram_addr(ugeth->
>  
> rx_bd_ring_offset[j]);
> @@ -2510,7 +2510,7 @@ static int ucc_geth_startup(struct ucc_geth_private 
> *ugeth)
> ugeth->tx_glbl_pram_offset =
> qe_muram_alloc(sizeof(struct ucc_geth_tx_global_pram),
>UCC_GETH_TX_GLOBAL_PRAM_ALIGNMENT);
> -   if (IS_ERR_VALUE(ugeth->tx_glbl_pram_offset)) {
> +   if (IS_ERR_VALUE((unsigned long)(int)ugeth->tx_glbl_pram_offset)) {
> if (netif_msg_ifup(ugeth))
> pr_err("Can not allocate DPRAM memory for 
> p_tx_glbl_pram\n");
> return -ENOMEM;
> @@ -2530,7 +2530,7 @@ static int ucc_geth_startup(struct ucc_geth_private 
> *ugeth)
>sizeof(struct ucc_geth_thread_data_tx) +
>32 * (numThreadsTxNumerical == 1),
>UCC_GETH_THREAD_DATA_ALIGNMENT);
> -   if (IS_ERR_VALUE(ugeth->thread_dat_tx_offset)) {
> +   if (IS_ERR_VALUE((unsigned long)(int)ugeth->thread_dat_tx_offset)) {
> if (netif_msg_ifup(ugeth))
> pr_err("Can not allocate DPRAM memory for 
> p_thread_data_tx\n");
> return -ENOMEM;
> @@ -2557,7 +2557,7 @@ static int ucc_geth_startup(struct ucc_geth_private 
> *ugeth)
> qe_muram_alloc(ug_info->numQueuesTx *
>sizeof(struct ucc_geth_send_queue_qd),
>UCC_GETH_SEND_QUEUE_QUEUE_DESCRIPTOR_ALIGNMENT);
> -   if (IS_ERR_VALUE(ugeth->send_q_mem_reg_offset)) {
> +   if (IS_ERR_VALUE((unsigned long)(int)ugeth->send_q_mem_reg_offset)) {
> if 

Re: [PATCH] net/ethernet/freescale: Fix incorrect IS_ERR_VALUE macro usages

[Li Yang]

On Tue, Nov 24, 2020 at 3:44 PM Li Yang  wrote:
>
> On Tue, Nov 24, 2020 at 12:24 AM Wei Li  wrote:
> >
> > IS_ERR_VALUE macro should be used only with unsigned long type.
> > Especially it works incorrectly with unsigned shorter types on
> > 64bit machines.
>
> This is truly a problem for the driver to run on 64-bit architectures.
> But from an earlier discussion
> https://patchwork.kernel.org/project/linux-kbuild/patch/1464384685-347275-1-git-send-email-a...@arndb.de/,
> the preferred solution would be removing the IS_ERR_VALUE() usage or
> make the values to be unsigned long.
>
> It looks like we are having a bigger problem with the 64-bit support
> for the driver that the offset variables can also be real pointers
> which cannot be held with 32-bit data types(when uf_info->bd_mem_part
> == MEM_PART_SYSTEM).  So actually we have to change these offsets to
> unsigned long, otherwise we are having more serious issues on 64-bit
> systems.  Are you willing to make such changes or you want us to deal
> with it?

Well, it looks like this hardware block was never integrated on a
64-bit SoC and will very likely to keep so.  So probably we can keep
the driver 32-bit only.  It is currently limited to PPC32 in Kconfig,
how did you build it for 64-bit?

>
> Regards,
> Leo
> >
> > Fixes: 4c35630ccda5 ("[POWERPC] Change rheap functions to use ulongs 
> > instead of pointers")
> > Signed-off-by: Wei Li 
> > ---
> >  drivers/net/ethernet/freescale/ucc_geth.c | 30 +++
> >  1 file changed, 15 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/ucc_geth.c 
> > b/drivers/net/ethernet/freescale/ucc_geth.c
> > index 714b501be7d0..8656d9be256a 100644
> > --- a/drivers/net/ethernet/freescale/ucc_geth.c
> > +++ b/drivers/net/ethernet/freescale/ucc_geth.c
> > @@ -286,7 +286,7 @@ static int fill_init_enet_entries(struct 
> > ucc_geth_private *ugeth,
> > else {
> > init_enet_offset =
> > qe_muram_alloc(thread_size, thread_alignment);
> > -   if (IS_ERR_VALUE(init_enet_offset)) {
> > +   if (IS_ERR_VALUE((unsigned 
> > long)(int)init_enet_offset)) {
> > if (netif_msg_ifup(ugeth))
> > pr_err("Can not allocate DPRAM 
> > memory\n");
> > qe_put_snum((u8) snum);
> > @@ -2223,7 +2223,7 @@ static int ucc_geth_alloc_tx(struct ucc_geth_private 
> > *ugeth)
> > ugeth->tx_bd_ring_offset[j] =
> > qe_muram_alloc(length,
> >UCC_GETH_TX_BD_RING_ALIGNMENT);
> > -   if (!IS_ERR_VALUE(ugeth->tx_bd_ring_offset[j]))
> > +   if (!IS_ERR_VALUE((unsigned 
> > long)(int)ugeth->tx_bd_ring_offset[j]))
> > ugeth->p_tx_bd_ring[j] =
> > (u8 __iomem *) qe_muram_addr(ugeth->
> >  
> > tx_bd_ring_offset[j]);
> > @@ -2300,7 +2300,7 @@ static int ucc_geth_alloc_rx(struct ucc_geth_private 
> > *ugeth)
> > ugeth->rx_bd_ring_offset[j] =
> > qe_muram_alloc(length,
> >UCC_GETH_RX_BD_RING_ALIGNMENT);
> > -   if (!IS_ERR_VALUE(ugeth->rx_bd_ring_offset[j]))
> > +   if (!IS_ERR_VALUE((unsigned 
> > long)(int)ugeth->rx_bd_ring_offset[j]))
> > ugeth->p_rx_bd_ring[j] =
> > (u8 __iomem *) qe_muram_addr(ugeth->
> >  
> > rx_bd_ring_offset[j]);
> > @@ -2510,7 +2510,7 @@ static int ucc_geth_startup(struct ucc_geth_private 
> > *ugeth)
> > ugeth->tx_glbl_pram_offset =
> > qe_muram_alloc(sizeof(struct ucc_geth_tx_global_pram),
> >UCC_GETH_TX_GLOBAL_PRAM_ALIGNMENT);
> > -   if (IS_ERR_VALUE(ugeth->tx_glbl_pram_offset)) {
> > +   if (IS_ERR_VALUE((unsigned long)(int)ugeth->tx_glbl_pram_offset)) {
> > if (netif_msg_ifup(ugeth))
> > pr_err("Can not allocate DPRAM memory for 
> > p_tx_glbl_pram\n");
> > return -ENOMEM;
> > @@ -2530,7 +2530,7 @@ static int ucc_geth_startup(st

Re: [PATCH 25/25] soc: fsl: qbman: qman: Remove unused variable 'dequeue_wq'

[Li Yang]

Hi Roy,

On Tue, Nov 3, 2020 at 9:31 AM Lee Jones  wrote:
>
> Fixes the following W=1 kernel build warning(s):
>
>  drivers/soc/fsl/qbman/qman.c: In function ‘qman_shutdown_fq’:
>  drivers/soc/fsl/qbman/qman.c:2700:8: warning: variable ‘dequeue_wq’ set but 
> not used [-Wunused-but-set-variable]
>
> Cc: Li Yang 
> Cc: YueHaibing 
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Lee Jones 
> ---
>  drivers/soc/fsl/qbman/qman.c | 8 ++--
>  1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/soc/fsl/qbman/qman.c b/drivers/soc/fsl/qbman/qman.c
> index 9888a70618730..62b182c3a8b04 100644
> --- a/drivers/soc/fsl/qbman/qman.c
> +++ b/drivers/soc/fsl/qbman/qman.c
> @@ -2622,7 +2622,7 @@ int qman_shutdown_fq(u32 fqid)
> union qm_mc_command *mcc;
> union qm_mc_result *mcr;
> int orl_empty, drain = 0, ret = 0;
> -   u32 channel, wq, res;
> +   u32 channel, res;
> u8 state;
>
> p = get_affine_portal();
> @@ -2655,7 +2655,7 @@ int qman_shutdown_fq(u32 fqid)
> DPAA_ASSERT((mcr->verb & QM_MCR_VERB_MASK) == QM_MCR_VERB_QUERYFQ);
> /* Need to store these since the MCR gets reused */
> channel = qm_fqd_get_chan(>queryfq.fqd);
> -   wq = qm_fqd_get_wq(>queryfq.fqd);
> +   qm_fqd_get_wq(>queryfq.fqd);

This probably is not needed also.

>
> if (channel < qm_channel_pool1) {
> channel_portal = get_portal_for_channel(channel);
> @@ -2697,7 +2697,6 @@ int qman_shutdown_fq(u32 fqid)
>  * to dequeue from the channel the FQ is scheduled on
>  */
> int found_fqrn = 0;
> -   u16 dequeue_wq = 0;
>
> /* Flag that we need to drain FQ */
> drain = 1;
> @@ -2705,11 +2704,8 @@ int qman_shutdown_fq(u32 fqid)
> if (channel >= qm_channel_pool1 &&
> channel < qm_channel_pool1 + 15) {
> /* Pool channel, enable the bit in the portal 
> */
> -   dequeue_wq = (channel -
> - qm_channel_pool1 + 1)<<4 | wq;
> } else if (channel < qm_channel_pool1) {
> /* Dedicated channel */
> -   dequeue_wq = wq;

With these gone, these if statements seem to be redundant.  Can you
propose an additional patch to further cleanup the code here?  Thanks.

> } else {
> dev_err(dev, "Can't recover FQ 0x%x, ch: 
> 0x%x",
> fqid, channel);
> --
> 2.25.1
>

Re: [PATCH v3] soc: fsl: dpio: Get the cpumask through cpumask_of(cpu)

[Li Yang]

On Mon, Oct 19, 2020 at 9:15 PM Yi Wang  wrote:
>
> From: Hao Si 
>
> The local variable 'cpumask_t mask' is in the stack memory, and its address
> is assigned to 'desc->affinity' in 'irq_set_affinity_hint()'.
> But the memory area where this variable is located is at risk of being
> modified.
>
> During LTP testing, the following error was generated:
>
> Unable to handle kernel paging request at virtual address 12e9b790
> Mem abort info:
>   ESR = 0x9607
>   Exception class = DABT (current EL), IL = 32 bits
>   SET = 0, FnV = 0
>   EA = 0, S1PTW = 0
> Data abort info:
>   ISV = 0, ISS = 0x0007
>   CM = 0, WnR = 0
> swapper pgtable: 4k pages, 48-bit VAs, pgdp = 75ac5e07
> [12e9b790] pgd=0027dbffe003, pud=0027dbffd003,
> pmd=0027b6d61003, pte=
> Internal error: Oops: 9607 [#1] PREEMPT SMP
> Modules linked in: xt_conntrack
> Process read_all (pid: 20171, stack limit = 0x44ea4095)
> CPU: 14 PID: 20171 Comm: read_all Tainted: GB   W
> Hardware name: NXP Layerscape LX2160ARDB (DT)
> pstate: 8085 (Nzcv daIf -PAN -UAO)
> pc : irq_affinity_hint_proc_show+0x54/0xb0
> lr : irq_affinity_hint_proc_show+0x4c/0xb0
> sp : 1138bc10
> x29: 1138bc10 x28: d131d1e0
> x27: 007000c0 x26: 8025b9480dc0
> x25: 8025b9480da8 x24: 03ff
> x23: 8027334f8300 x22: 80272e97d000
> x21: 80272e97d0b0 x20: 8025b9480d80
> x19: 09a49000 x18: 
> x17:  x16: 
> x15:  x14: 
> x13:  x12: 0040
> x11:  x10: 802735b79b88
> x9 :  x8 : 
> x7 : 09a49848 x6 : 0003
> x5 :  x4 : 08157d6c
> x3 : 1138bc10 x2 : 12e9b790
> x1 :  x0 : 
> Call trace:
>  irq_affinity_hint_proc_show+0x54/0xb0
>  seq_read+0x1b0/0x440
>  proc_reg_read+0x80/0xd8
>  __vfs_read+0x60/0x178
>  vfs_read+0x94/0x150
>  ksys_read+0x74/0xf0
>  __arm64_sys_read+0x24/0x30
>  el0_svc_common.constprop.0+0xd8/0x1a0
>  el0_svc_handler+0x34/0x88
>  el0_svc+0x10/0x14
> Code: f9001bbf 943e0732 f94066c2 b462 (f9400041)
> ---[ end trace b495bdcb0b3b732b ]---
> Kernel panic - not syncing: Fatal exception
> SMP: stopping secondary CPUs
> SMP: failed to stop secondary CPUs 0,2-4,6,8,11,13-15
> Kernel Offset: disabled
> CPU features: 0x0,21006008
> Memory Limit: none
> ---[ end Kernel panic - not syncing: Fatal exception ]---
>
> Fix it by using 'cpumask_of(cpu)' to get the cpumask.
>
> Signed-off-by: Hao Si 
> Signed-off-by: Lin Chen 
> Signed-off-by: Yi Wang 

Applied for fix.  Thanks.

> ---
> v3: Use cpumask_of(cpu) to get the pre-defined cpumask in the static
> cpu_bit_bitmap array.
> v2: Place 'cpumask_t mask' in the driver's private data and while at it,
> rename it to cpu_mask.
>
>  drivers/soc/fsl/dpio/dpio-driver.c | 5 +
>  1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/soc/fsl/dpio/dpio-driver.c 
> b/drivers/soc/fsl/dpio/dpio-driver.c
> index 7b642c3..7f397b4 100644
> --- a/drivers/soc/fsl/dpio/dpio-driver.c
> +++ b/drivers/soc/fsl/dpio/dpio-driver.c
> @@ -95,7 +95,6 @@ static int register_dpio_irq_handlers(struct fsl_mc_device 
> *dpio_dev, int cpu)
>  {
> int error;
> struct fsl_mc_device_irq *irq;
> -   cpumask_t mask;
>
> irq = dpio_dev->irqs[0];
> error = devm_request_irq(_dev->dev,
> @@ -112,9 +111,7 @@ static int register_dpio_irq_handlers(struct 
> fsl_mc_device *dpio_dev, int cpu)
> }
>
> /* set the affinity hint */
> -   cpumask_clear();
> -   cpumask_set_cpu(cpu, );
> -   if (irq_set_affinity_hint(irq->msi_desc->irq, ))
> +   if (irq_set_affinity_hint(irq->msi_desc->irq, cpumask_of(cpu)))
> dev_err(_dev->dev,
> "irq_set_affinity failed irq %d cpu %d\n",
> irq->msi_desc->irq, cpu);
> --
> 2.15.2

Re: [PATCH 00/25] Rid W=1 warnings in SoC

[Li Yang]

On Tue, Nov 3, 2020 at 9:29 AM Lee Jones  wrote:
>
> This set is part of a larger effort attempting to clean-up W=1
> kernel builds, which are currently overwhelmingly riddled with
> niggly little warnings.
>
> Lee Jones (25):

>   soc: fsl: dpio: qbman-portal: Fix a bunch of kernel-doc misdemeanours
>   soc: fsl: qe: qe_common: Fix misnamed function attribute 'addr'
>   soc: fsl: qbman: qman: Remove unused variable 'dequeue_wq'

The above are applied for next.  Thanks.

Regards,
Leo
>
>  drivers/soc/bcm/brcmstb/pm/pm-arm.c  |  2 +
>  drivers/soc/fsl/dpio/qbman-portal.c  | 18 +--
>  drivers/soc/fsl/qbman/qman.c |  8 +--
>  drivers/soc/fsl/qe/qe_common.c   |  2 +-
>  drivers/soc/qcom/kryo-l2-accessors.c |  2 +-
>  drivers/soc/qcom/llcc-qcom.c |  2 +-
>  drivers/soc/qcom/qcom-geni-se.c  |  5 +-
>  drivers/soc/qcom/qcom_aoss.c |  4 +-
>  drivers/soc/qcom/rpmh.c  |  2 +-
>  drivers/soc/qcom/rpmhpd.c|  3 ++
>  drivers/soc/qcom/smem.c  |  3 +-
>  drivers/soc/qcom/smp2p.c |  3 +-
>  drivers/soc/qcom/smsm.c  |  4 +-
>  drivers/soc/qcom/wcnss_ctrl.c|  8 +--
>  drivers/soc/rockchip/io-domain.c |  3 --
>  drivers/soc/samsung/s3c-pm-check.c   |  2 +-
>  drivers/soc/tegra/fuse/speedo-tegra124.c |  7 ++-
>  drivers/soc/tegra/fuse/speedo-tegra210.c |  8 +--
>  drivers/soc/ti/k3-ringacc.c  |  1 +
>  drivers/soc/ti/knav_dma.c|  2 +-
>  drivers/soc/ti/knav_qmss_queue.c | 62 
>  drivers/soc/ti/pm33xx.c  |  4 +-
>  drivers/soc/ti/wkup_m3_ipc.c |  8 ++-
>  23 files changed, 86 insertions(+), 77 deletions(-)
>
> Cc: act 
> Cc: Andy Gross 
> Cc: bcm-kernel-feedback-l...@broadcom.com
> Cc: Ben Dooks 
> Cc: Bjorn Andersson 
> Cc: Cyril Chemparathy 
> Cc: Dan Malek 
> Cc: Dave Gerlach 
> Cc: Doug Anderson 
> Cc: Florian Fainelli 
> Cc: Heiko Stuebner 
> Cc: Jonathan Hunter 
> Cc: Krzysztof Kozlowski 
> Cc: Liam Girdwood 
> Cc: linux-arm-...@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-rockc...@lists.infradead.org
> Cc: linux-samsung-...@vger.kernel.org
> Cc: linux-te...@vger.kernel.org
> Cc: Li Yang 
> Cc: Mark Brown 
> Cc: Qiang Zhao 
> Cc: "Rafael J. Wysocki" 
> Cc: Roy Pledge 
> Cc: Sandeep Nair 
> Cc: Santosh Shilimkar 
> Cc: Scott Wood 
> Cc: "Software, Inc" 
> Cc: Thierry Reding 
> Cc: Vitaly Bordug 
> Cc: YueHaibing 
>
> --
> 2.25.1
>

Re: [PATCH v6 04/49] soc: fsl: qe: introduce qe_io{read, write}* wrappers

[Li Yang]

On Tue, Jan 19, 2021 at 11:35 AM Christophe Leroy
 wrote:
>
> Hi Rasmus,
>
> Le 28/11/2019 à 15:55, Rasmus Villemoes a écrit :
> > The QUICC engine drivers use the powerpc-specific out_be32() etc. In
> > order to allow those drivers to build for other architectures, those
> > must be replaced by iowrite32be(). However, on powerpc, out_be32() is
> > a simple inline function while iowrite32be() is out-of-line. So in
> > order not to introduce a performance regression on powerpc when making
> > the drivers work on other architectures, introduce qe_io* helpers.
> >
> > Also define the qe_{clr,set,clrset}bits* helpers in terms of these new
> > macros.
>
> Since commit 
> https://github.com/linuxppc/linux/commit/894fa235eb4ca0bfa692dbe4932c2f940cdc8c1e
> ioread/iowrite wrappers are also inlined on PPC32, so this commit can now be 
> reverted.

Yes.  That will be great.

>
> Christophe
>
> >
> > Reviewed-by: Timur Tabi 
> > Signed-off-by: Rasmus Villemoes 
> > ---
> >   include/soc/fsl/qe/qe.h | 34 +-
> >   1 file changed, 25 insertions(+), 9 deletions(-)
> >
> > diff --git a/include/soc/fsl/qe/qe.h b/include/soc/fsl/qe/qe.h
> > index a1aa4eb28f0c..9cac04c692fd 100644
> > --- a/include/soc/fsl/qe/qe.h
> > +++ b/include/soc/fsl/qe/qe.h
> > @@ -241,21 +241,37 @@ static inline int qe_alive_during_sleep(void)
> >   #define qe_muram_offset cpm_muram_offset
> >   #define qe_muram_dma cpm_muram_dma
> >
> > -#define qe_setbits_be32(_addr, _v) iowrite32be(ioread32be(_addr) |  (_v), 
> > (_addr))
> > -#define qe_clrbits_be32(_addr, _v) iowrite32be(ioread32be(_addr) & ~(_v), 
> > (_addr))
> > +#ifdef CONFIG_PPC32
> > +#define qe_iowrite8(val, addr) out_8(addr, val)
> > +#define qe_iowrite16be(val, addr)  out_be16(addr, val)
> > +#define qe_iowrite32be(val, addr)  out_be32(addr, val)
> > +#define qe_ioread8(addr)   in_8(addr)
> > +#define qe_ioread16be(addr)in_be16(addr)
> > +#define qe_ioread32be(addr)in_be32(addr)
> > +#else
> > +#define qe_iowrite8(val, addr) iowrite8(val, addr)
> > +#define qe_iowrite16be(val, addr)  iowrite16be(val, addr)
> > +#define qe_iowrite32be(val, addr)  iowrite32be(val, addr)
> > +#define qe_ioread8(addr)   ioread8(addr)
> > +#define qe_ioread16be(addr)ioread16be(addr)
> > +#define qe_ioread32be(addr)ioread32be(addr)
> > +#endif
> > +
> > +#define qe_setbits_be32(_addr, _v) qe_iowrite32be(qe_ioread32be(_addr) |  
> > (_v), (_addr))
> > +#define qe_clrbits_be32(_addr, _v) qe_iowrite32be(qe_ioread32be(_addr) & 
> > ~(_v), (_addr))
> >
> > -#define qe_setbits_be16(_addr, _v) iowrite16be(ioread16be(_addr) |  (_v), 
> > (_addr))
> > -#define qe_clrbits_be16(_addr, _v) iowrite16be(ioread16be(_addr) & ~(_v), 
> > (_addr))
> > +#define qe_setbits_be16(_addr, _v) qe_iowrite16be(qe_ioread16be(_addr) |  
> > (_v), (_addr))
> > +#define qe_clrbits_be16(_addr, _v) qe_iowrite16be(qe_ioread16be(_addr) & 
> > ~(_v), (_addr))
> >
> > -#define qe_setbits_8(_addr, _v) iowrite8(ioread8(_addr) |  (_v), (_addr))
> > -#define qe_clrbits_8(_addr, _v) iowrite8(ioread8(_addr) & ~(_v), (_addr))
> > +#define qe_setbits_8(_addr, _v) qe_iowrite8(qe_ioread8(_addr) |  (_v), 
> > (_addr))
> > +#define qe_clrbits_8(_addr, _v) qe_iowrite8(qe_ioread8(_addr) & ~(_v), 
> > (_addr))
> >
> >   #define qe_clrsetbits_be32(addr, clear, set) \
> > - iowrite32be((ioread32be(addr) & ~(clear)) | (set), (addr))
> > + qe_iowrite32be((qe_ioread32be(addr) & ~(clear)) | (set), (addr))
> >   #define qe_clrsetbits_be16(addr, clear, set) \
> > - iowrite16be((ioread16be(addr) & ~(clear)) | (set), (addr))
> > + qe_iowrite16be((qe_ioread16be(addr) & ~(clear)) | (set), (addr))
> >   #define qe_clrsetbits_8(addr, clear, set) \
> > - iowrite8((ioread8(addr) & ~(clear)) | (set), (addr))
> > + qe_iowrite8((qe_ioread8(addr) & ~(clear)) | (set), (addr))
> >
> >   /* Structure that defines QE firmware binary files.
> >*
> >

[PATCH] usb: gadget: fsl: properly remove remnant of MXC support

[Li Yang]

Commit a390bef7db1f ("usb: gadget: fsl_mxc_udc: Remove the driver")
didn't remove all the MXC related stuff which can cause build problem
for LS1021 when enabled again in Kconfig.  This patch remove all the
remnants.

Signed-off-by: Li Yang 
---
 drivers/usb/gadget/udc/fsl_udc_core.c | 36 +--
 drivers/usb/gadget/udc/fsl_usb2_udc.h | 19 --
 2 files changed, 6 insertions(+), 49 deletions(-)

diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c 
b/drivers/usb/gadget/udc/fsl_udc_core.c
index 2b357b3f64c0..29fcb9b461d7 100644
--- a/drivers/usb/gadget/udc/fsl_udc_core.c
+++ b/drivers/usb/gadget/udc/fsl_udc_core.c
@@ -36,7 +36,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 
 #include 
@@ -323,13 +322,11 @@ static int dr_controller_setup(struct fsl_udc *udc)
fsl_writel(tmp, _regs->endptctrl[ep_num]);
}
/* Config control enable i/o output, cpu endian register */
-#ifndef CONFIG_ARCH_MXC
if (udc->pdata->have_sysif_regs) {
ctrl = __raw_readl(_sys_regs->control);
ctrl |= USB_CTRL_IOENB;
__raw_writel(ctrl, _sys_regs->control);
}
-#endif
 
 #if defined(CONFIG_PPC32) && !defined(CONFIG_NOT_COHERENT_CACHE)
/* Turn on cache snooping hardware, since some PowerPC platforms
@@ -2153,7 +2150,6 @@ static int fsl_proc_read(struct seq_file *m, void *v)
tmp_reg = fsl_readl(_regs->endpointprime);
seq_printf(m, "EP Prime Reg = [0x%x]\n\n", tmp_reg);
 
-#ifndef CONFIG_ARCH_MXC
if (udc->pdata->have_sysif_regs) {
tmp_reg = usb_sys_regs->snoop1;
seq_printf(m, "Snoop1 Reg : = [0x%x]\n\n", tmp_reg);
@@ -2161,7 +2157,6 @@ static int fsl_proc_read(struct seq_file *m, void *v)
tmp_reg = usb_sys_regs->control;
seq_printf(m, "General Control Reg : = [0x%x]\n\n", tmp_reg);
}
-#endif
 
/* --fsl_udc, fsl_ep, fsl_request structure information - */
ep = >eps[0];
@@ -2412,28 +2407,21 @@ static int fsl_udc_probe(struct platform_device *pdev)
 */
if (pdata->init && pdata->init(pdev)) {
ret = -ENODEV;
-   goto err_iounmap_noclk;
+   goto err_iounmap;
}
 
/* Set accessors only after pdata->init() ! */
fsl_set_accessors(pdata);
 
-#ifndef CONFIG_ARCH_MXC
if (pdata->have_sysif_regs)
usb_sys_regs = (void *)dr_regs + USB_DR_SYS_OFFSET;
-#endif
-
-   /* Initialize USB clocks */
-   ret = fsl_udc_clk_init(pdev);
-   if (ret < 0)
-   goto err_iounmap_noclk;
 
/* Read Device Controller Capability Parameters register */
dccparams = fsl_readl(_regs->dccparams);
if (!(dccparams & DCCPARAMS_DC)) {
ERR("This SOC doesn't support device role\n");
ret = -ENODEV;
-   goto err_iounmap;
+   goto err_exit;
}
/* Get max device endpoints */
/* DEN is bidirectional ep number, max_ep doubles the number */
@@ -2442,7 +2430,7 @@ static int fsl_udc_probe(struct platform_device *pdev)
ret = platform_get_irq(pdev, 0);
if (ret <= 0) {
ret = ret ? : -ENODEV;
-   goto err_iounmap;
+   goto err_exit;
}
udc_controller->irq = ret;
 
@@ -2451,7 +2439,7 @@ static int fsl_udc_probe(struct platform_device *pdev)
if (ret != 0) {
ERR("cannot request irq %d err %d\n",
udc_controller->irq, ret);
-   goto err_iounmap;
+   goto err_exit;
}
 
/* Initialize the udc structure including QH member and other member */
@@ -2467,10 +2455,6 @@ static int fsl_udc_probe(struct platform_device *pdev)
dr_controller_setup(udc_controller);
}
 
-   ret = fsl_udc_clk_finalize(pdev);
-   if (ret)
-   goto err_free_irq;
-
/* Setup gadget structure */
udc_controller->gadget.ops = _gadget_ops;
udc_controller->gadget.max_speed = USB_SPEED_HIGH;
@@ -2530,11 +2514,10 @@ static int fsl_udc_probe(struct platform_device *pdev)
dma_pool_destroy(udc_controller->td_pool);
 err_free_irq:
free_irq(udc_controller->irq, udc_controller);
-err_iounmap:
+err_exit:
if (pdata->exit)
pdata->exit(pdev);
-   fsl_udc_clk_release();
-err_iounmap_noclk:
+err_iounmap:
iounmap(dr_regs);
 err_release_mem_region:
if (pdata->operating_mode == FSL_USB2_DR_DEVICE)
@@ -2561,8 +2544,6 @@ static int fsl_udc_remove(struct platform_device *pdev)
udc_controller->done = 
usb_del_gadget_udc(_controller->gadget);
 
-   fsl_udc_clk_release();
-
  

Re: [PATCH] soc/fsl: qbman: fix conflicting alignment attributes

[Li Yang]

On Tue, Mar 23, 2021 at 8:17 AM Arnd Bergmann  wrote:
>
> From: Arnd Bergmann 
>
> When building with W=1, gcc points out that the __packed attribute
> on struct qm_eqcr_entry conflicts with the 8-byte alignment
> attribute on struct qm_fd inside it:
>
> drivers/soc/fsl/qbman/qman.c:189:1: error: alignment 1 of 'struct 
> qm_eqcr_entry' is less than 8 [-Werror=packed-not-aligned]
>
> I assume that the alignment attribute is the correct one, and
> that qm_eqcr_entry cannot actually be unaligned in memory,
> so add the same alignment on the outer struct.
>
> Fixes: c535e923bb97 ("soc/fsl: Introduce DPAA 1.x QMan device driver")
> Signed-off-by: Arnd Bergmann 
> ---
>  drivers/soc/fsl/qbman/qman.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/soc/fsl/qbman/qman.c b/drivers/soc/fsl/qbman/qman.c
> index a1b9be1d105a..fde4edd83c14 100644
> --- a/drivers/soc/fsl/qbman/qman.c
> +++ b/drivers/soc/fsl/qbman/qman.c
> @@ -186,7 +186,7 @@ struct qm_eqcr_entry {
> __be32 tag;
> struct qm_fd fd;
> u8 __reserved3[32];
> -} __packed;
> +} __packed __aligned(8);

The EQCR structure is actually aligned on 64-byte from the manual.
But probably 8 is enough to let the compiler not complain.

>  #define QM_EQCR_VERB_VBIT  0x80
>  #define QM_EQCR_VERB_CMD_MASK  0x61/* but only one value; */
>  #define QM_EQCR_VERB_CMD_ENQUEUE   0x01
> --
> 2.29.2
>

Re: [PATCH v6] soc: fsl: enable acpi support in RCPM driver

[Li Yang]

On Fri, Mar 12, 2021 at 2:56 AM Ran Wang  wrote:
>
> From: Peng Ma 
>
> This patch enables ACPI support in RCPM driver.
>
> Signed-off-by: Peng Ma 
> Signed-off-by: Ran Wang 
> ---
> Change in v6:
>  - Remove copyright udpate to rebase on latest mainline
>
> Change in v5:
>  - Fix panic when dev->of_node is null
>
> Change in v4:
>  - Make commit subject more accurate
>  - Remove unrelated new blank line
>
> Change in v3:
>  - Add #ifdef CONFIG_ACPI for acpi_device_id
>  - Rename rcpm_acpi_imx_ids to rcpm_acpi_ids
>
> Change in v2:
>  - Update acpi_device_id to fix conflict with other driver
>
>  drivers/soc/fsl/rcpm.c | 18 --
>  1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/soc/fsl/rcpm.c b/drivers/soc/fsl/rcpm.c
> index 4ace28cab314..7aa997b932d1 100644
> --- a/drivers/soc/fsl/rcpm.c
> +++ b/drivers/soc/fsl/rcpm.c
> @@ -13,6 +13,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  #define RCPM_WAKEUP_CELL_MAX_SIZE  7
>
> @@ -78,10 +79,14 @@ static int rcpm_pm_prepare(struct device *dev)
> "fsl,rcpm-wakeup", value,
> rcpm->wakeup_cells + 1);
>
> -   /*  Wakeup source should refer to current rcpm device */
> -   if (ret || (np->phandle != value[0]))
> +   if (ret)
> continue;
>
> +   if (is_of_node(dev->fwnode))
> +   /*  Should refer to current rcpm device */
> +   if (np->phandle != value[0])
> +   continue;

It looks like that we assume that in the ACPI scenario there will only
be one RCPM controller and all devices are controlled by this single
PM controller.  This probably is true for all existing SoCs with a
RCPM.  But since the driver tried to support multiple RCPMs, maybe we
should continue to support multiple RCPM controllers or at least
mention that in the comment.

> +
> /* Property "#fsl,rcpm-wakeup-cells" of rcpm node defines the
>  * number of IPPDEXPCR register cells, and "fsl,rcpm-wakeup"
>  * of wakeup source IP contains an integer array:  @@ -172,10 +177,19 @@ static const struct of_device_id rcpm_of_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, rcpm_of_match);
>
> +#ifdef CONFIG_ACPI
> +static const struct acpi_device_id rcpm_acpi_ids[] = {
> +   {"NXP0015",},
> +   { }
> +};
> +MODULE_DEVICE_TABLE(acpi, rcpm_acpi_ids);
> +#endif
> +
>  static struct platform_driver rcpm_driver = {
> .driver = {
> .name = "rcpm",
> .of_match_table = rcpm_of_match,
> +   .acpi_match_table = ACPI_PTR(rcpm_acpi_ids),
> .pm = _pm_ops,
> },
> .probe = rcpm_probe,
> --
> 2.25.1
>

Re: [PATCH v1 0/3] Remove qe_io{read,write}* IO accessors

[Li Yang]

On Sat, Mar 6, 2021 at 12:11 PM Christophe Leroy
 wrote:
>
> Commit 6ac9b61786cc ("soc: fsl: qe: introduce qe_io{read,write}*
> wrappers") added specific I/O accessors for qe because at that
> time ioread/iowrite functions were sub-optimal on powerpc/32
> compared to the architecture specific in_/out_ IO accessors.
>
> But as ioread/iowrite accessors are now equivalent since
> commit 894fa235eb4c ("powerpc: inline iomap accessors"),
> use them in order to allow removal of the qe specific ones.
>
> Christophe Leroy (3):
>   soc: fsl: qe: replace qe_io{read,write}* wrappers by generic
> io{read,write}*
>   tty: serial: ucc_uart: replace qe_io{read,write}* wrappers by generic
> io{read,write}*
>   Revert "soc: fsl: qe: introduce qe_io{read,write}* wrappers"

Series applied.  Thanks.

>
>  drivers/soc/fsl/qe/gpio.c |  20 +++---
>  drivers/soc/fsl/qe/qe.c   |  24 +++
>  drivers/soc/fsl/qe/qe_ic.c|   4 +-
>  drivers/soc/fsl/qe/qe_io.c|  36 +-
>  drivers/soc/fsl/qe/ucc_fast.c |  68 +--
>  drivers/soc/fsl/qe/ucc_slow.c |  42 ++--
>  drivers/tty/serial/ucc_uart.c | 124 +-
>  include/soc/fsl/qe/qe.h   |  34 +++---
>  8 files changed, 168 insertions(+), 184 deletions(-)
>
> --
> 2.25.0
>

Re: [PATCH v7] soc: fsl: enable acpi support in RCPM driver

[Li Yang]

On Wed, Apr 7, 2021 at 9:58 PM Ran Wang  wrote:
>
> From: Peng Ma 
>
> This patch enables ACPI support in RCPM driver.
>
> Signed-off-by: Peng Ma 
> Signed-off-by: Ran Wang 

Applied for next.  Thanks.

> ---
> Change in v7:
>  - Update comment for checking RCPM node which refferred to
>
> Change in v6:
>  - Remove copyright udpate to rebase on latest mainline
>
> Change in v5:
>  - Fix panic when dev->of_node is null
>
> Change in v4:
>  - Make commit subject more accurate
>  - Remove unrelated new blank line
>
> Change in v3:
>  - Add #ifdef CONFIG_ACPI for acpi_device_id
>  - Rename rcpm_acpi_imx_ids to rcpm_acpi_ids
>
> Change in v2:
>  - Update acpi_device_id to fix conflict with other driver
>
>  drivers/soc/fsl/rcpm.c | 24 ++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/soc/fsl/rcpm.c b/drivers/soc/fsl/rcpm.c
> index 4ace28cab314..90d3f4060b0c 100644
> --- a/drivers/soc/fsl/rcpm.c
> +++ b/drivers/soc/fsl/rcpm.c
> @@ -13,6 +13,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  #define RCPM_WAKEUP_CELL_MAX_SIZE  7
>
> @@ -78,10 +79,20 @@ static int rcpm_pm_prepare(struct device *dev)
> "fsl,rcpm-wakeup", value,
> rcpm->wakeup_cells + 1);
>
> -   /*  Wakeup source should refer to current rcpm device */
> -   if (ret || (np->phandle != value[0]))
> +   if (ret)
> continue;
>
> +   /*
> +* For DT mode, would handle devices with "fsl,rcpm-wakeup"
> +* pointing to the current RCPM node.
> +*
> +* For ACPI mode, currently we assume there is only one
> +* RCPM controller existing.
> +*/
> +   if (is_of_node(dev->fwnode))
> +   if (np->phandle != value[0])
> +   continue;
> +
> /* Property "#fsl,rcpm-wakeup-cells" of rcpm node defines the
>  * number of IPPDEXPCR register cells, and "fsl,rcpm-wakeup"
>  * of wakeup source IP contains an integer array:  @@ -172,10 +183,19 @@ static const struct of_device_id rcpm_of_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, rcpm_of_match);
>
> +#ifdef CONFIG_ACPI
> +static const struct acpi_device_id rcpm_acpi_ids[] = {
> +   {"NXP0015",},
> +   { }
> +};
> +MODULE_DEVICE_TABLE(acpi, rcpm_acpi_ids);
> +#endif
> +
>  static struct platform_driver rcpm_driver = {
> .driver = {
> .name = "rcpm",
> .of_match_table = rcpm_of_match,
> +   .acpi_match_table = ACPI_PTR(rcpm_acpi_ids),
> .pm = _pm_ops,
> },
> .probe = rcpm_probe,
> --
> 2.25.1
>

Re: [PATCH] soc: fsl: qe: fix static checker warning

[Li Yang]

On Wed, Aug 11, 2021 at 2:10 AM Maxim Kochetkov  wrote:
>
> The patch be7ecbd240b2: "soc: fsl: qe: convert QE interrupt
> controller to platform_device" from Aug 3, 2021, leads to the
> following static checker warning:
>
> drivers/soc/fsl/qe/qe_ic.c:438 qe_ic_init()
> warn: unsigned 'qe_ic->virq_low' is never less than zero.
>
> In old variant irq_of_parse_and_map() returns zero if failed so
> unsigned int for virq_high/virq_low was ok.
> In new variant platform_get_irq() returns negative error codes
> if failed so we need to use int for virq_high/virq_low.
>
> Also simplify high_handler checking and remove the curly braces
> to make checkpatch happy.
>
> Fixes: be7ecbd240b2 ("soc: fsl: qe: convert QE interrupt controller to 
> platform_device")
> Signed-off-by: Maxim Kochetkov 
> Reported-by: Dan Carpenter 
> ---
>  drivers/soc/fsl/qe/qe_ic.c | 9 -
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/soc/fsl/qe/qe_ic.c b/drivers/soc/fsl/qe/qe_ic.c
> index e710d554425d..bff34ee2150a 100644
> --- a/drivers/soc/fsl/qe/qe_ic.c
> +++ b/drivers/soc/fsl/qe/qe_ic.c
> @@ -54,8 +54,8 @@ struct qe_ic {
> struct irq_chip hc_irq;
>
> /* VIRQ numbers of QE high/low irqs */
> -   unsigned int virq_high;
> -   unsigned int virq_low;
> +   int virq_high;
> +   int virq_low;
>  };
>
>  /*
> @@ -435,9 +435,8 @@ static int qe_ic_init(struct platform_device *pdev)
> qe_ic->virq_high = platform_get_irq(pdev, 0);
> qe_ic->virq_low = platform_get_irq(pdev, 1);
>
> -   if (qe_ic->virq_low < 0) {
> +   if (qe_ic->virq_low < 0)

Probably should be <= 0 here.

> return -ENODEV;
> -   }
>
> if (qe_ic->virq_high != qe_ic->virq_low) {

Probably we should check if qe_ic->virq_high > 0 here if we rely on
this to decide whether to set the handler later.

Applied with the above changes.  Thanks

> low_handler = qe_ic_cascade_low;
> @@ -459,7 +458,7 @@ static int qe_ic_init(struct platform_device *pdev)
> irq_set_handler_data(qe_ic->virq_low, qe_ic);
> irq_set_chained_handler(qe_ic->virq_low, low_handler);
>
> -   if (qe_ic->virq_high && qe_ic->virq_high != qe_ic->virq_low) {
> +   if (high_handler) {
> irq_set_handler_data(qe_ic->virq_high, qe_ic);
> irq_set_chained_handler(qe_ic->virq_high, high_handler);
> }
> --
> 2.31.1
>

Re: [PATCH] soc: fsl: qe: convert QE interrupt controller to platform_device

[Li Yang]

On Mon, Jul 5, 2021 at 6:12 AM Maxim Kochetkov  wrote:
>
> Since 5.13 QE's ucc nodes can't get interrupts from devicetree:
>
> ucc@2000 {
> cell-index = <1>;
> reg = <0x2000 0x200>;
> interrupts = <32>;
> interrupt-parent = <>;
> };
>
> Now fw_devlink expects driver to create and probe a struct device
> for interrupt controller.
>
> So lets convert this driver to simple platform_device with probe().
>
> [1] - 
> https://lore.kernel.org/lkml/CAGETcx9PiX==mlxb9po8myyk6u2vhpvwtmsa5nkd-ywh5xh...@mail.gmail.com
> Fixes: e590474768f1 ("driver core: Set fw_devlink=on by default")
> Fixes: ea718c699055 ("Revert "Revert "driver core: Set fw_devlink=on by 
> default""")
> Signed-off-by: Maxim Kochetkov 
> ---
>  drivers/soc/fsl/qe/qe_ic.c | 38 +++---
>  1 file changed, 23 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/soc/fsl/qe/qe_ic.c b/drivers/soc/fsl/qe/qe_ic.c
> index 3f711c1a0996..03d291376895 100644
> --- a/drivers/soc/fsl/qe/qe_ic.c
> +++ b/drivers/soc/fsl/qe/qe_ic.c
> @@ -23,6 +23,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -404,27 +405,28 @@ static void qe_ic_cascade_muxed_mpic(struct irq_desc 
> *desc)
> chip->irq_eoi(>irq_data);
>  }
>
> -static void __init qe_ic_init(struct device_node *node)
> +static int qe_ic_init(struct platform_device *pdev)
>  {
> void (*low_handler)(struct irq_desc *desc);
> void (*high_handler)(struct irq_desc *desc);
> struct qe_ic *qe_ic;
> struct resource res;
> +   struct device_node *node = pdev->dev.of_node;
> u32 ret;
>
> ret = of_address_to_resource(node, 0, );
> if (ret)
> -   return;
> +   return -ENODEV;
>
> qe_ic = kzalloc(sizeof(*qe_ic), GFP_KERNEL);
> if (qe_ic == NULL)
> -   return;
> +   return -ENOMEM;
>
> qe_ic->irqhost = irq_domain_add_linear(node, NR_QE_IC_INTS,
>_ic_host_ops, qe_ic);
> if (qe_ic->irqhost == NULL) {
> kfree(qe_ic);
> -   return;
> +   return -ENODEV;
> }
>
> qe_ic->regs = ioremap(res.start, resource_size());
> @@ -437,7 +439,7 @@ static void __init qe_ic_init(struct device_node *node)
> if (!qe_ic->virq_low) {
> printk(KERN_ERR "Failed to map QE_IC low IRQ\n");
> kfree(qe_ic);
> -   return;
> +   return -ENODEV;
> }
> if (qe_ic->virq_high != qe_ic->virq_low) {
> low_handler = qe_ic_cascade_low;
> @@ -456,20 +458,26 @@ static void __init qe_ic_init(struct device_node *node)
> irq_set_handler_data(qe_ic->virq_high, qe_ic);
> irq_set_chained_handler(qe_ic->virq_high, high_handler);
> }
> +   return 0;
>  }
> +static const struct of_device_id qe_ic_ids[] = {
> +   { .compatible = "fsl,qe-ic"},
> +   { .compatible = "qeic"},

>From the original code, this should be type = "qeic".  It is not
defined in current binding but probably needed for backward
compatibility.

It would be great if you can also deal with the comments from Dan too.  Thanks.

> +   {},
> +};
>
> -static int __init qe_ic_of_init(void)
> +static struct platform_driver qe_ic_driver =
>  {
> -   struct device_node *np;
> +   .driver = {
> +   .name   = "qe-ic",
> +   .of_match_table = qe_ic_ids,
> +   },
> +   .probe  = qe_ic_init,
> +};
>
> -   np = of_find_compatible_node(NULL, NULL, "fsl,qe-ic");
> -   if (!np) {
> -   np = of_find_node_by_type(NULL, "qeic");
> -   if (!np)
> -   return -ENODEV;
> -   }
> -   qe_ic_init(np);
> -   of_node_put(np);
> +static int __init qe_ic_of_init(void)
> +{
> +   platform_driver_register(_ic_driver);
> return 0;
>  }
>  subsys_initcall(qe_ic_of_init);
> --
> 2.31.1
>

Re: [PATCH] soc: fsl: guts: Fix a resource leak in the error handling path of 'fsl_guts_probe()'

[Li Yang]

On Wed, Aug 18, 2021 at 4:23 PM Christophe JAILLET
 wrote:
>
> If an error occurs after 'of_find_node_by_path()', the reference taken for
> 'root' will never be released and some memory will leak.

Thanks for finding this.  This truly is a problem.

>
> Instead of adding an error handling path and modifying all the
> 'return -SOMETHING' into 'goto errorpath', use 'devm_add_action_or_reset()'
> to release the reference when needed.
>
> Simplify the remove function accordingly.
>
> As an extra benefit, the 'root' global variable can now be removed as well.
>
> Fixes: 3c0d64e867ed ("soc: fsl: guts: reuse machine name from device tree")
> Signed-off-by: Christophe JAILLET 
> ---
> Compile tested only
> ---
>  drivers/soc/fsl/guts.c | 16 ++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
> index d5e9a5f2c087..4d9476c7b87c 100644
> --- a/drivers/soc/fsl/guts.c
> +++ b/drivers/soc/fsl/guts.c
> @@ -28,7 +28,6 @@ struct fsl_soc_die_attr {
>  static struct guts *guts;
>  static struct soc_device_attribute soc_dev_attr;
>  static struct soc_device *soc_dev;
> -static struct device_node *root;
>
>
>  /* SoC die attribute definition for QorIQ platform */
> @@ -136,14 +135,23 @@ static u32 fsl_guts_get_svr(void)
> return svr;
>  }
>
> +static void fsl_guts_put_root(void *data)
> +{
> +   struct device_node *root = data;
> +
> +   of_node_put(root);
> +}
> +
>  static int fsl_guts_probe(struct platform_device *pdev)
>  {
> struct device_node *np = pdev->dev.of_node;
> struct device *dev = >dev;
> +   struct device_node *root;
> struct resource *res;
> const struct fsl_soc_die_attr *soc_die;
> const char *machine;
> u32 svr;
> +   int ret;
>
> /* Initialize guts */
> guts = devm_kzalloc(dev, sizeof(*guts), GFP_KERNEL);
> @@ -159,6 +167,10 @@ static int fsl_guts_probe(struct platform_device *pdev)
>
> /* Register soc device */
> root = of_find_node_by_path("/");
> +   ret = devm_add_action_or_reset(dev, fsl_guts_put_root, root);
> +   if (ret)
> +   return ret;

We probably only need to hold the reference when we do get "machine"
from the device tree, otherwise we can put it directly.

Or maybe we just maintain a local copy of string machine which means
we can release the reference right away?

> +
> if (of_property_read_string(root, "model", ))
> of_property_read_string_index(root, "compatible", 0, 
> );
> if (machine)
> @@ -197,7 +209,7 @@ static int fsl_guts_probe(struct platform_device *pdev)
>  static int fsl_guts_remove(struct platform_device *dev)
>  {
> soc_device_unregister(soc_dev);
> -   of_node_put(root);
> +
> return 0;
>  }
>
> --
> 2.30.2
>

Re: [PATCH 1/2] soc: fsl: guts: Make use of the helper function devm_platform_ioremap_resource()

[Li Yang]

On Wed, Sep 8, 2021 at 2:19 AM Cai Huoqing  wrote:
>
> Use the devm_platform_ioremap_resource() helper instead of
> calling platform_get_resource() and devm_ioremap_resource()
> separately
>
> Signed-off-by: Cai Huoqing 

Applied for next.  Thanks.

> ---
>  drivers/soc/fsl/guts.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
> index d5e9a5f2c087..072473a16f4d 100644
> --- a/drivers/soc/fsl/guts.c
> +++ b/drivers/soc/fsl/guts.c
> @@ -140,7 +140,6 @@ static int fsl_guts_probe(struct platform_device *pdev)
>  {
> struct device_node *np = pdev->dev.of_node;
> struct device *dev = >dev;
> -   struct resource *res;
> const struct fsl_soc_die_attr *soc_die;
> const char *machine;
> u32 svr;
> @@ -152,8 +151,7 @@ static int fsl_guts_probe(struct platform_device *pdev)
>
> guts->little_endian = of_property_read_bool(np, "little-endian");
>
> -   res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -   guts->regs = devm_ioremap_resource(dev, res);
> +   guts->regs = devm_platform_ioremap_resource(pdev, 0);
> if (IS_ERR(guts->regs))
> return PTR_ERR(guts->regs);
>
> --
> 2.25.1
>

Re: [PATCH 2/2] soc: fsl: rcpm: Make use of the helper function devm_platform_ioremap_resource()

[Li Yang]

On Wed, Sep 8, 2021 at 2:20 AM Cai Huoqing  wrote:
>
> Use the devm_platform_ioremap_resource() helper instead of
> calling platform_get_resource() and devm_ioremap_resource()
> separately
>
> Signed-off-by: Cai Huoqing 

Applied for next.  Thanks.

> ---
>  drivers/soc/fsl/rcpm.c | 7 +--
>  1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/drivers/soc/fsl/rcpm.c b/drivers/soc/fsl/rcpm.c
> index 90d3f4060b0c..3d0cae30c769 100644
> --- a/drivers/soc/fsl/rcpm.c
> +++ b/drivers/soc/fsl/rcpm.c
> @@ -146,7 +146,6 @@ static const struct dev_pm_ops rcpm_pm_ops = {
>  static int rcpm_probe(struct platform_device *pdev)
>  {
> struct device   *dev = >dev;
> -   struct resource *r;
> struct rcpm *rcpm;
> int ret;
>
> @@ -154,11 +153,7 @@ static int rcpm_probe(struct platform_device *pdev)
> if (!rcpm)
> return -ENOMEM;
>
> -   r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -   if (!r)
> -   return -ENODEV;
> -
> -   rcpm->ippdexpcr_base = devm_ioremap_resource(>dev, r);
> +   rcpm->ippdexpcr_base = devm_platform_ioremap_resource(pdev, 0);
> if (IS_ERR(rcpm->ippdexpcr_base)) {
> ret =  PTR_ERR(rcpm->ippdexpcr_base);
> return ret;
> --
> 2.25.1
>

Re: [PATCH v2] soc: fsl: dpio: instead smp_processor_id with raw_smp_processor_id

[Li Yang]

On Mon, Oct 18, 2021 at 9:46 PM  wrote:
>
> From: Meng Li 
>
> When enable debug kernel configs,there will be calltrace as below:
>
> BUG: using smp_processor_id() in preemptible [] code: swapper/0/1
> caller is debug_smp_processor_id+0x20/0x30
> CPU: 6 PID: 1 Comm: swapper/0 Not tainted 5.10.63-yocto-standard #1
> Hardware name: NXP Layerscape LX2160ARDB (DT)
> Call trace:
>  dump_backtrace+0x0/0x1a0
>  show_stack+0x24/0x30
>  dump_stack+0xf0/0x13c
>  check_preemption_disabled+0x100/0x110
>  debug_smp_processor_id+0x20/0x30
>  dpaa2_io_query_fq_count+0xdc/0x154
>  dpaa2_eth_stop+0x144/0x314
>  __dev_close_many+0xdc/0x160
>  __dev_change_flags+0xe8/0x220
>  dev_change_flags+0x30/0x70
>  ic_close_devs+0x50/0x78
>  ip_auto_config+0xed0/0xf10
>  do_one_initcall+0xac/0x460
>  kernel_init_freeable+0x30c/0x378
>  kernel_init+0x20/0x128
>  ret_from_fork+0x10/0x38
>
> Based on comment in the context, it doesn't matter whether
> preemption is disable or not. So, instead smp_processor_id()

s/instead/replace/g

> with raw_smp_processor_id() to avoid above call trace.
>
> v2:
> Remove the preempt_disable/enable() protection, instead smp_processor_id
> with raw_smp_processor_id.

The revision history information should go after the "---" below.

>
> Fixes: c89105c9b390 ("staging: fsl-mc: Move DPIO from staging to 
> drivers/soc/fsl")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Meng Li 

I helped to fix the issues I mentioned.  Applied for fix.  Thanks.

> ---
>  drivers/soc/fsl/dpio/dpio-service.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/soc/fsl/dpio/dpio-service.c 
> b/drivers/soc/fsl/dpio/dpio-service.c
> index 19f47ea9dab0..3050a534d42c 100644
> --- a/drivers/soc/fsl/dpio/dpio-service.c
> +++ b/drivers/soc/fsl/dpio/dpio-service.c
> @@ -59,7 +59,7 @@ static inline struct dpaa2_io *service_select_by_cpu(struct 
> dpaa2_io *d,
>  * potentially being migrated away.
>  */
> if (cpu < 0)
> -   cpu = smp_processor_id();
> +   cpu = raw_smp_processor_id();
>
> /* If a specific cpu was requested, pick it up immediately */
> return dpio_by_cpu[cpu];
> --
> 2.17.1
>

Re: [PATCH] driver: soc: dpio: use the whole functions to protect critical zone

[Li Yang]

On Mon, Oct 18, 2021 at 10:07 PM  wrote:
>
> From: Meng Li 
>
> In orininal code, use 2 function spin_lock() and local_irq_save() to
> protect the critical zone. But when enable the kernel debug config,
> there are below inconsistent lock state detected.
> 
> WARNING: inconsistent lock state
> 5.10.63-yocto-standard #1 Not tainted
> 
> inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
> lock_torture_wr/226 [HC0[0]:SC1[5]:HE1:SE0] takes:
> 002005b2dd80 (>access_spinlock){+.?.}-{3:3}, at: 
> qbman_swp_enqueue_multiple_mem_back+0x44/0x270
> {SOFTIRQ-ON-W} state was registered at:
>   lock_acquire.part.0+0xf8/0x250
>   lock_acquire+0x68/0x84
>   _raw_spin_lock+0x68/0x90
>   qbman_swp_enqueue_multiple_mem_back+0x44/0x270
>   ..
>   cryptomgr_test+0x38/0x60
>   kthread+0x158/0x164
>   ret_from_fork+0x10/0x38
> irq event stamp: 4498
> hardirqs last  enabled at (4498): [] 
> _raw_spin_unlock_irqrestore+0x90/0xb0
> hardirqs last disabled at (4497): [] 
> _raw_spin_lock_irqsave+0xd4/0xe0
> softirqs last  enabled at (4458): [] 
> __do_softirq+0x674/0x724
> softirqs last disabled at (4465): [] 
> __irq_exit_rcu+0x190/0x19c
>
> other info that might help us debug this:
>  Possible unsafe locking scenario:
>CPU0
>
>   lock(>access_spinlock);
>   
> lock(>access_spinlock);
>  *** DEADLOCK ***
>
> So, in order to avoid deadlock, use the whole functinos

s/functinos/functions/

> spin_lock_irqsave/spin_unlock_irqrestore() to protect critical zone.
>
> Fixes: 3b2abda7d28c ("soc: fsl: dpio: Replace QMAN array mode with ring mode 
> enqueue")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Meng Li 

Applied for fix.  Thanks.

> ---
>  drivers/soc/fsl/dpio/qbman-portal.c | 9 +++--
>  1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/soc/fsl/dpio/qbman-portal.c 
> b/drivers/soc/fsl/dpio/qbman-portal.c
> index 845e91416b58..a56dbe4de34f 100644
> --- a/drivers/soc/fsl/dpio/qbman-portal.c
> +++ b/drivers/soc/fsl/dpio/qbman-portal.c
> @@ -785,8 +785,7 @@ int qbman_swp_enqueue_multiple_mem_back(struct qbman_swp 
> *s,
> int i, num_enqueued = 0;
> unsigned long irq_flags;
>
> -   spin_lock(>access_spinlock);
> -   local_irq_save(irq_flags);
> +   spin_lock_irqsave(>access_spinlock, irq_flags);
>
> half_mask = (s->eqcr.pi_ci_mask>>1);
> full_mask = s->eqcr.pi_ci_mask;
> @@ -797,8 +796,7 @@ int qbman_swp_enqueue_multiple_mem_back(struct qbman_swp 
> *s,
> s->eqcr.available = qm_cyc_diff(s->eqcr.pi_ring_size,
> eqcr_ci, s->eqcr.ci);
> if (!s->eqcr.available) {
> -   local_irq_restore(irq_flags);
> -   spin_unlock(>access_spinlock);
> +   spin_unlock_irqrestore(>access_spinlock, 
> irq_flags);
> return 0;
> }
> }
> @@ -837,8 +835,7 @@ int qbman_swp_enqueue_multiple_mem_back(struct qbman_swp 
> *s,
> dma_wmb();
> qbman_write_register(s, QBMAN_CINH_SWP_EQCR_PI,
> (QB_RT_BIT)|(s->eqcr.pi)|s->eqcr.pi_vb);
> -   local_irq_restore(irq_flags);
> -   spin_unlock(>access_spinlock);
> +   spin_unlock_irqrestore(>access_spinlock, irq_flags);
>
> return num_enqueued;
>  }
> --
> 2.17.1
>

Re: [PATCH 1/5] dt-bindings: memory: fsl: convert ifc binding to yaml schema

[Li Yang]

On Mon, Oct 4, 2021 at 4:32 AM Krzysztof Kozlowski
 wrote:
>
> On 01/10/2021 18:17, Li Yang wrote:
> > On Fri, Oct 1, 2021 at 5:01 AM Krzysztof Kozlowski
> >  wrote:
> >>
>
> (...)
>
> >>> +
> >>> +  interrupts:
> >>> +minItems: 1
> >>> +maxItems: 2
> >>> +description: |
> >>> +  IFC may have one or two interrupts.  If two interrupt specifiers 
> >>> are
> >>> +  present, the first is the "common" interrupt (CM_EVTER_STAT), and 
> >>> the
> >>> +  second is the NAND interrupt (NAND_EVTER_STAT).  If there is only 
> >>> one,
> >>> +  that interrupt reports both types of event.
> >>> +
> >>> +  little-endian:
> >>> +$ref: '/schemas/types.yaml#/definitions/flag'
> >>
> >> type: boolean
> >
> > It will not have a true or false value, but only present or not.  Is
> > the boolean type taking care of this too?
>
> boolean is for a property which does not accept values and true/false
> depends on its presence.
> See:
> Documentation/devicetree/bindings/phy/lantiq,vrx200-pcie-phy.yaml
> Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml

>From the dtschema/schemas/types.yaml file, flag type is defined as:
  flag:
oneOf:
  - type: boolean
const: true
  - type: 'null'

It looks like more than the boolean type itself.  But if the standard
boolean type is actually the same as the flag type we defined.
Shouldn't we remove the custom flag type then?

Regards,
Leo

Re: [PATCH 2/5] memory: fsl_ifc: populate child devices without relying on simple-bus

[Li Yang]

On Mon, Oct 4, 2021 at 12:14 PM Rob Herring  wrote:
>
> On Thu, Sep 30, 2021 at 07:09:21PM -0500, Li Yang wrote:
> > After we update the binding to not use simple-bus compatible for the
> > controller, we need the driver to populate the child devices explicitly.
> >
> > Signed-off-by: Li Yang 
> > ---
> >  drivers/memory/fsl_ifc.c | 9 +
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/drivers/memory/fsl_ifc.c b/drivers/memory/fsl_ifc.c
> > index d062c2f8250f..251d713cd50b 100644
> > --- a/drivers/memory/fsl_ifc.c
> > +++ b/drivers/memory/fsl_ifc.c
> > @@ -88,6 +88,7 @@ static int fsl_ifc_ctrl_remove(struct platform_device 
> > *dev)
> >  {
> >   struct fsl_ifc_ctrl *ctrl = dev_get_drvdata(>dev);
> >
> > + of_platform_depopulate(>dev);
> >   free_irq(ctrl->nand_irq, ctrl);
> >   free_irq(ctrl->irq, ctrl);
> >
> > @@ -285,6 +286,14 @@ static int fsl_ifc_ctrl_probe(struct platform_device 
> > *dev)
> >   }
> >   }
> >
> > + /* legacy dts may still use "simple-bus" compatible */
> > + if (!of_device_is_compatible(dev->dev.of_node, "simple-bus")) {
> > + ret = of_platform_populate(dev->dev.of_node, NULL, NULL,
> > + >dev);
>
> There's no need to make this conditional. of_platform_populate() is safe
> to call multiple times. If that doesn't work, it's a bug.

I think that it is probably an optimization to avoid re-populate the
bus for legacy device trees.  But it might be cleaner to just
re-populate anyway?

Regards,
Leo

[PATCH 1/5] dt-bindings: memory: fsl: convert ifc binding to yaml schema

[Li Yang]

Convert the txt binding to yaml format and add description.  Drop the
"simple-bus" compatible string from the example and not allowed by the
binding any more.  This will help to enforce the correct probe order
between parent device and child devices, but will require the ifc driver
to probe the child devices to work properly.

Signed-off-by: Li Yang 
---
updates from previous submission:
- Drop "simple-bus" from binding and only "fsl,ifc" as compatible
- Fix one identiation problem of "reg"
- Add type restriction to "little-endian" property

 .../bindings/memory-controllers/fsl/ifc.txt   |  82 ---
 .../bindings/memory-controllers/fsl/ifc.yaml  | 137 ++
 2 files changed, 137 insertions(+), 82 deletions(-)
 delete mode 100644 
Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt
 create mode 100644 
Documentation/devicetree/bindings/memory-controllers/fsl/ifc.yaml

diff --git a/Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt 
b/Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt
deleted file mode 100644
index 89427b018ba7..
--- a/Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt
+++ /dev/null
@@ -1,82 +0,0 @@
-Integrated Flash Controller
-
-Properties:
-- name : Should be ifc
-- compatible : should contain "fsl,ifc". The version of the integrated
-   flash controller can be found in the IFC_REV register at
-   offset zero.
-
-- #address-cells : Should be either two or three.  The first cell is the
-   chipselect number, and the remaining cells are the
-   offset into the chipselect.
-- #size-cells : Either one or two, depending on how large each chipselect
-can be.
-- reg : Offset and length of the register set for the device
-- interrupts: IFC may have one or two interrupts.  If two interrupt
-  specifiers are present, the first is the "common"
-  interrupt (CM_EVTER_STAT), and the second is the NAND
-  interrupt (NAND_EVTER_STAT).  If there is only one,
-  that interrupt reports both types of event.
-
-- little-endian : If this property is absent, the big-endian mode will
-  be in use as default for registers.
-
-- ranges : Each range corresponds to a single chipselect, and covers
-   the entire access window as configured.
-
-Child device nodes describe the devices connected to IFC such as NOR (e.g.
-cfi-flash) and NAND (fsl,ifc-nand). There might be board specific devices
-like FPGAs, CPLDs, etc.
-
-Example:
-
-   ifc@ffe1e000 {
-   compatible = "fsl,ifc", "simple-bus";
-   #address-cells = <2>;
-   #size-cells = <1>;
-   reg = <0x0 0xffe1e000 0 0x2000>;
-   interrupts = <16 2 19 2>;
-   little-endian;
-
-   /* NOR, NAND Flashes and CPLD on board */
-   ranges = <0x0 0x0 0x0 0xee00 0x0200
- 0x1 0x0 0x0 0xffa0 0x0001
- 0x3 0x0 0x0 0xffb0 0x0002>;
-
-   flash@0,0 {
-   #address-cells = <1>;
-   #size-cells = <1>;
-   compatible = "cfi-flash";
-   reg = <0x0 0x0 0x200>;
-   bank-width = <2>;
-   device-width = <1>;
-
-   partition@0 {
-   /* 32MB for user data */
-   reg = <0x0 0x0200>;
-   label = "NOR Data";
-   };
-   };
-
-   flash@1,0 {
-   #address-cells = <1>;
-   #size-cells = <1>;
-   compatible = "fsl,ifc-nand";
-   reg = <0x1 0x0 0x1>;
-
-   partition@0 {
-   /* This location must not be altered  */
-   /* 1MB for u-boot Bootloader Image */
-   reg = <0x0 0x0010>;
-   label = "NAND U-Boot Image";
-   read-only;
-   };
-   };
-
-   cpld@3,0 {
-   #address-cells = <1>;
-   #size-cells = <1>;
-   compatible = "fsl,p1010rdb-cpld";
-   reg = <0x3 0x0 0x01f>;
-   };
-   };
diff --git a/Documentation/devicetree/bindings/memory-controllers/fsl/ifc.yaml 
b/Documentation/devicetree/bindings/memory-controllers/fsl/ifc.yaml
new file mode 100644
index ..19871ce39fe3
--- /dev/n

[PATCH 3/5] ARM: dts: ls1021a: remove "simple-bus" compatible from ifc node

[Li Yang]

The binding of ifc device has been updated.  Update dts to match
accordingly.

Signed-off-by: Li Yang 
---
 arch/arm/boot/dts/ls1021a.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/ls1021a.dtsi b/arch/arm/boot/dts/ls1021a.dtsi
index 7cddc05825a1..4aeb804e61b1 100644
--- a/arch/arm/boot/dts/ls1021a.dtsi
+++ b/arch/arm/boot/dts/ls1021a.dtsi
@@ -123,7 +123,7 @@ msi2: msi-controller@1570e08 {
};
 
ifc: ifc@153 {
-   compatible = "fsl,ifc", "simple-bus";
+   compatible = "fsl,ifc";
reg = <0x0 0x153 0x0 0x1>;
interrupts = ;
status = "disabled";
-- 
2.25.1

[PATCH 0/5] convert ifc binding to yaml and drop "simple-bus"

[Li Yang]

Convert the ifc binding to yaml schema, in the mean while remove the
"simple-bus" compatible from the binding to make sure ifc device probes
before any of the child devices.  Update the driver and existing DTSes
accordingly.

DTS changes should be merged together with the driver/binding changes
if DTS maintainer is ok with it or after the driver changes are applied.

Li Yang (5):
  dt-bindings: memory: fsl: convert ifc binding to yaml schema
  memory: fsl_ifc: populate child devices without relying on simple-bus
  ARM: dts: ls1021a: remove "simple-bus" compatible from ifc node
  arm64: dts: remove "simple-bus" compatible from ifc node
  powerpc/mpc85xx: remove "simple-bus" compatible from ifc node

 .../bindings/memory-controllers/fsl/ifc.txt   |  82 ---
 .../bindings/memory-controllers/fsl/ifc.yaml  | 137 ++
 arch/arm/boot/dts/ls1021a.dtsi|   2 +-
 .../arm64/boot/dts/freescale/fsl-ls1043a.dtsi |   2 +-
 .../arm64/boot/dts/freescale/fsl-ls1046a.dtsi |   2 +-
 .../arm64/boot/dts/freescale/fsl-ls1088a.dtsi |   2 +-
 .../arm64/boot/dts/freescale/fsl-ls208xa.dtsi |   2 +-
 arch/powerpc/boot/dts/fsl/b4si-post.dtsi  |   2 +-
 arch/powerpc/boot/dts/fsl/bsc9131si-post.dtsi |   2 +-
 arch/powerpc/boot/dts/fsl/bsc9132si-post.dtsi |   2 +-
 arch/powerpc/boot/dts/fsl/c293si-post.dtsi|   2 +-
 arch/powerpc/boot/dts/fsl/p1010si-post.dtsi   |   2 +-
 arch/powerpc/boot/dts/fsl/t1023si-post.dtsi   |   2 +-
 arch/powerpc/boot/dts/fsl/t1040si-post.dtsi   |   2 +-
 arch/powerpc/boot/dts/fsl/t2081si-post.dtsi   |   2 +-
 arch/powerpc/boot/dts/fsl/t4240si-post.dtsi   |   2 +-
 drivers/memory/fsl_ifc.c  |   9 ++
 17 files changed, 160 insertions(+), 96 deletions(-)
 delete mode 100644 
Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt
 create mode 100644 
Documentation/devicetree/bindings/memory-controllers/fsl/ifc.yaml

-- 
2.25.1

[PATCH 4/5] arm64: dts: remove "simple-bus" compatible from ifc node

[Li Yang]

The binding of ifc device has been updated.  Update dts to match
accordingly.

Signed-off-by: Li Yang 
---
 arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 2 +-
 arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi | 2 +-
 arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 2 +-
 arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi 
b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
index d856a1f45da1..b19a44204df8 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
@@ -435,7 +435,7 @@ dcfg: dcfg@1ee {
};
 
ifc: ifc@153 {
-   compatible = "fsl,ifc", "simple-bus";
+   compatible = "fsl,ifc";
reg = <0x0 0x153 0x0 0x1>;
interrupts = <0 43 0x4>;
};
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi 
b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
index 41904b8bc85e..1b065f134bd6 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
@@ -283,7 +283,7 @@ ddr: memory-controller@108 {
};
 
ifc: ifc@153 {
-   compatible = "fsl,ifc", "simple-bus";
+   compatible = "fsl,ifc";
reg = <0x0 0x153 0x0 0x1>;
interrupts = ;
status = "disabled";
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi 
b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
index f8f252fdc039..2b1b0adfd340 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
@@ -398,7 +398,7 @@ gpio3: gpio@233 {
};
 
ifc: ifc@224 {
-   compatible = "fsl,ifc", "simple-bus";
+   compatible = "fsl,ifc";
reg = <0x0 0x224 0x0 0x2>;
interrupts = <0 21 IRQ_TYPE_LEVEL_HIGH>;
little-endian;
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi 
b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
index d8590d080c5e..3679f1707048 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi
@@ -1037,7 +1037,7 @@ i2c3: i2c@203 {
};
 
ifc: ifc@224 {
-   compatible = "fsl,ifc", "simple-bus";
+   compatible = "fsl,ifc";
reg = <0x0 0x224 0x0 0x2>;
interrupts = <0 21 0x4>; /* Level high type */
little-endian;
-- 
2.25.1

[PATCH 5/5] powerpc/mpc85xx: remove "simple-bus" compatible from ifc node

[Li Yang]

The binding of ifc device has been updated.  Update dts to match
accordingly.

Signed-off-by: Li Yang 
---
 arch/powerpc/boot/dts/fsl/b4si-post.dtsi  | 2 +-
 arch/powerpc/boot/dts/fsl/bsc9131si-post.dtsi | 2 +-
 arch/powerpc/boot/dts/fsl/bsc9132si-post.dtsi | 2 +-
 arch/powerpc/boot/dts/fsl/c293si-post.dtsi| 2 +-
 arch/powerpc/boot/dts/fsl/p1010si-post.dtsi   | 2 +-
 arch/powerpc/boot/dts/fsl/t1023si-post.dtsi   | 2 +-
 arch/powerpc/boot/dts/fsl/t1040si-post.dtsi   | 2 +-
 arch/powerpc/boot/dts/fsl/t2081si-post.dtsi   | 2 +-
 arch/powerpc/boot/dts/fsl/t4240si-post.dtsi   | 2 +-
 9 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/boot/dts/fsl/b4si-post.dtsi 
b/arch/powerpc/boot/dts/fsl/b4si-post.dtsi
index 4f044b41a776..fb3200b006ad 100644
--- a/arch/powerpc/boot/dts/fsl/b4si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/b4si-post.dtsi
@@ -50,7 +50,7 @@ _pfdr {
  {
#address-cells = <2>;
#size-cells = <1>;
-   compatible = "fsl,ifc", "simple-bus";
+   compatible = "fsl,ifc";
interrupts = <25 2 0 0>;
 };
 
diff --git a/arch/powerpc/boot/dts/fsl/bsc9131si-post.dtsi 
b/arch/powerpc/boot/dts/fsl/bsc9131si-post.dtsi
index 2a677fd323eb..5c53cee8755f 100644
--- a/arch/powerpc/boot/dts/fsl/bsc9131si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/bsc9131si-post.dtsi
@@ -35,7 +35,7 @@
  {
#address-cells = <2>;
#size-cells = <1>;
-   compatible = "fsl,ifc", "simple-bus";
+   compatible = "fsl,ifc";
interrupts = <16 2 0 0 20 2 0 0>;
 };
 
diff --git a/arch/powerpc/boot/dts/fsl/bsc9132si-post.dtsi 
b/arch/powerpc/boot/dts/fsl/bsc9132si-post.dtsi
index b8e0edd1ac69..4da451e000d9 100644
--- a/arch/powerpc/boot/dts/fsl/bsc9132si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/bsc9132si-post.dtsi
@@ -35,7 +35,7 @@
  {
#address-cells = <2>;
#size-cells = <1>;
-   compatible = "fsl,ifc", "simple-bus";
+   compatible = "fsl,ifc";
/* FIXME: Test whether interrupts are split */
interrupts = <16 2 0 0 20 2 0 0>;
 };
diff --git a/arch/powerpc/boot/dts/fsl/c293si-post.dtsi 
b/arch/powerpc/boot/dts/fsl/c293si-post.dtsi
index bec0fc36849d..ee3b45806ee3 100644
--- a/arch/powerpc/boot/dts/fsl/c293si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/c293si-post.dtsi
@@ -35,7 +35,7 @@
  {
#address-cells = <2>;
#size-cells = <1>;
-   compatible = "fsl,ifc", "simple-bus";
+   compatible = "fsl,ifc";
interrupts = <19 2 0 0>;
 };
 
diff --git a/arch/powerpc/boot/dts/fsl/p1010si-post.dtsi 
b/arch/powerpc/boot/dts/fsl/p1010si-post.dtsi
index b540e58ff79e..2d2550729dcc 100644
--- a/arch/powerpc/boot/dts/fsl/p1010si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/p1010si-post.dtsi
@@ -35,7 +35,7 @@
  {
#address-cells = <2>;
#size-cells = <1>;
-   compatible = "fsl,ifc", "simple-bus";
+   compatible = "fsl,ifc";
interrupts = <16 2 0 0 19 2 0 0>;
 };
 
diff --git a/arch/powerpc/boot/dts/fsl/t1023si-post.dtsi 
b/arch/powerpc/boot/dts/fsl/t1023si-post.dtsi
index d552044c5afc..c15a49df66e1 100644
--- a/arch/powerpc/boot/dts/fsl/t1023si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/t1023si-post.dtsi
@@ -52,7 +52,7 @@ _pfdr {
  {
#address-cells = <2>;
#size-cells = <1>;
-   compatible = "fsl,ifc", "simple-bus";
+   compatible = "fsl,ifc";
interrupts = <25 2 0 0>;
 };
 
diff --git a/arch/powerpc/boot/dts/fsl/t1040si-post.dtsi 
b/arch/powerpc/boot/dts/fsl/t1040si-post.dtsi
index f58eb820eb5e..38703e58dd09 100644
--- a/arch/powerpc/boot/dts/fsl/t1040si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/t1040si-post.dtsi
@@ -52,7 +52,7 @@ _pfdr {
  {
#address-cells = <2>;
#size-cells = <1>;
-   compatible = "fsl,ifc", "simple-bus";
+   compatible = "fsl,ifc";
interrupts = <25 2 0 0>;
 };
 
diff --git a/arch/powerpc/boot/dts/fsl/t2081si-post.dtsi 
b/arch/powerpc/boot/dts/fsl/t2081si-post.dtsi
index ecbb447920bc..58ef8bf6045c 100644
--- a/arch/powerpc/boot/dts/fsl/t2081si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/t2081si-post.dtsi
@@ -50,7 +50,7 @@ _pfdr {
  {
#address-cells = <2>;
#size-cells = <1>;
-   compatible = "fsl,ifc", "simple-bus";
+   compatible = "fsl,ifc";
interrupts = <25 2 0 0>;
 };
 
diff --git a/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi 
b/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi
index fcac73486d48..65f3e17c0d41 100644
--- a/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/t4240si-post.dtsi
@@ -50,7 +50,7 @@ _pfdr {
  {
#address-cells = <2>;
#size-cells = <1>;
-   compatible = "fsl,ifc", "simple-bus";
+   compatible = "fsl,ifc";
interrupts = <25 2 0 0>;
 };
 
-- 
2.25.1

[PATCH 2/5] memory: fsl_ifc: populate child devices without relying on simple-bus

[Li Yang]

After we update the binding to not use simple-bus compatible for the
controller, we need the driver to populate the child devices explicitly.

Signed-off-by: Li Yang 
---
 drivers/memory/fsl_ifc.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/drivers/memory/fsl_ifc.c b/drivers/memory/fsl_ifc.c
index d062c2f8250f..251d713cd50b 100644
--- a/drivers/memory/fsl_ifc.c
+++ b/drivers/memory/fsl_ifc.c
@@ -88,6 +88,7 @@ static int fsl_ifc_ctrl_remove(struct platform_device *dev)
 {
struct fsl_ifc_ctrl *ctrl = dev_get_drvdata(>dev);
 
+   of_platform_depopulate(>dev);
free_irq(ctrl->nand_irq, ctrl);
free_irq(ctrl->irq, ctrl);
 
@@ -285,6 +286,14 @@ static int fsl_ifc_ctrl_probe(struct platform_device *dev)
}
}
 
+   /* legacy dts may still use "simple-bus" compatible */
+   if (!of_device_is_compatible(dev->dev.of_node, "simple-bus")) {
+   ret = of_platform_populate(dev->dev.of_node, NULL, NULL,
+   >dev);
+   if (ret)
+   goto err_nandirq;
+   }
+
return 0;
 
 err_nandirq:
-- 
2.25.1

Re: [PATCH 1/5] dt-bindings: memory: fsl: convert ifc binding to yaml schema

[Li Yang]

On Fri, Oct 1, 2021 at 5:01 AM Krzysztof Kozlowski
 wrote:
>
> On 01/10/2021 02:09, Li Yang wrote:
> > Convert the txt binding to yaml format and add description.  Drop the
> > "simple-bus" compatible string from the example and not allowed by the
> > binding any more.  This will help to enforce the correct probe order
> > between parent device and child devices, but will require the ifc driver
> > to probe the child devices to work properly.
> >
> > Signed-off-by: Li Yang 
> > ---
> > updates from previous submission:
> > - Drop "simple-bus" from binding and only "fsl,ifc" as compatible
> > - Fix one identiation problem of "reg"
> > - Add type restriction to "little-endian" property
> >
> >  .../bindings/memory-controllers/fsl/ifc.txt   |  82 ---
> >  .../bindings/memory-controllers/fsl/ifc.yaml  | 137 ++
> >  2 files changed, 137 insertions(+), 82 deletions(-)
> >  delete mode 100644 
> > Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt
> >  create mode 100644 
> > Documentation/devicetree/bindings/memory-controllers/fsl/ifc.yaml
> >
> > diff --git 
> > a/Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt 
> > b/Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt
> > deleted file mode 100644
> > index 89427b018ba7..
> > --- a/Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt
> > +++ /dev/null
> > @@ -1,82 +0,0 @@
> > -Integrated Flash Controller
> > -
> > -Properties:
> > -- name : Should be ifc
> > -- compatible : should contain "fsl,ifc". The version of the integrated
> > -   flash controller can be found in the IFC_REV register at
> > -   offset zero.
> > -
> > -- #address-cells : Should be either two or three.  The first cell is the
> > -   chipselect number, and the remaining cells are the
> > -   offset into the chipselect.
> > -- #size-cells : Either one or two, depending on how large each chipselect
> > -can be.
> > -- reg : Offset and length of the register set for the device
> > -- interrupts: IFC may have one or two interrupts.  If two interrupt
> > -  specifiers are present, the first is the "common"
> > -  interrupt (CM_EVTER_STAT), and the second is the NAND
> > -  interrupt (NAND_EVTER_STAT).  If there is only one,
> > -  that interrupt reports both types of event.
> > -
> > -- little-endian : If this property is absent, the big-endian mode will
> > -  be in use as default for registers.
> > -
> > -- ranges : Each range corresponds to a single chipselect, and covers
> > -   the entire access window as configured.
> > -
> > -Child device nodes describe the devices connected to IFC such as NOR (e.g.
> > -cfi-flash) and NAND (fsl,ifc-nand). There might be board specific devices
> > -like FPGAs, CPLDs, etc.
> > -
> > -Example:
> > -
> > - ifc@ffe1e000 {
> > - compatible = "fsl,ifc", "simple-bus";
> > - #address-cells = <2>;
> > - #size-cells = <1>;
> > - reg = <0x0 0xffe1e000 0 0x2000>;
> > - interrupts = <16 2 19 2>;
> > - little-endian;
> > -
> > - /* NOR, NAND Flashes and CPLD on board */
> > - ranges = <0x0 0x0 0x0 0xee00 0x0200
> > -   0x1 0x0 0x0 0xffa0 0x0001
> > -   0x3 0x0 0x0 0xffb0 0x0002>;
> > -
> > - flash@0,0 {
> > - #address-cells = <1>;
> > - #size-cells = <1>;
> > - compatible = "cfi-flash";
> > - reg = <0x0 0x0 0x200>;
> > - bank-width = <2>;
> > - device-width = <1>;
> > -
> > - partition@0 {
> > - /* 32MB for user data */
> > - reg = <0x0 0x0200>;
> > - label = "NOR Data";
> > - };
> > - };
> > -
> > - flash@1,0 {
> > - #address-cells = <1>;
> > - #size-cells = <1>;
> > - compatible = "fsl,ifc

Re: [PATCH 1/5] dt-bindings: memory: fsl: convert ifc binding to yaml schema

[Li Yang]

On Fri, Oct 1, 2021 at 8:18 AM Rob Herring  wrote:
>
> On Thu, 30 Sep 2021 19:09:20 -0500, Li Yang wrote:
> > Convert the txt binding to yaml format and add description.  Drop the
> > "simple-bus" compatible string from the example and not allowed by the
> > binding any more.  This will help to enforce the correct probe order
> > between parent device and child devices, but will require the ifc driver
> > to probe the child devices to work properly.
> >
> > Signed-off-by: Li Yang 
> > ---
> > updates from previous submission:
> > - Drop "simple-bus" from binding and only "fsl,ifc" as compatible
> > - Fix one identiation problem of "reg"
> > - Add type restriction to "little-endian" property
> >
> >  .../bindings/memory-controllers/fsl/ifc.txt   |  82 ---
> >  .../bindings/memory-controllers/fsl/ifc.yaml  | 137 ++
> >  2 files changed, 137 insertions(+), 82 deletions(-)
> >  delete mode 100644 
> > Documentation/devicetree/bindings/memory-controllers/fsl/ifc.txt
> >  create mode 100644 
> > Documentation/devicetree/bindings/memory-controllers/fsl/ifc.yaml
> >
>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> Documentation/devicetree/bindings/memory-controllers/fsl/ifc.example.dt.yaml:0:0:
>  /example-0/soc/ifc@ffe1e000/flash@1,0: failed to match any schema with 
> compatible: ['fsl,ifc-nand']
> Documentation/devicetree/bindings/memory-controllers/fsl/ifc.example.dt.yaml:0:0:
>  /example-0/soc/ifc@ffe1e000/cpld@3,0: failed to match any schema with 
> compatible: ['fsl,p1010rdb-cpld']

These are defined in other bindings, but unfortunately they are not
converted to yaml format yet.

>
> doc reference errors (make refcheckdocs):
>
> See https://patchwork.ozlabs.org/patch/1535102
>
> This check can fail if there are any dependencies. The base for a patch
> series is generally the most recent rc1.
>
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
>
> pip3 install dtschema --upgrade
>
> Please check and re-submit.
>

Re: [PATCH 0/5] convert ifc binding to yaml and drop "simple-bus"

[Li Yang]

On Fri, Oct 1, 2021 at 4:46 AM Krzysztof Kozlowski
 wrote:
>
> On 01/10/2021 02:09, Li Yang wrote:
> > Convert the ifc binding to yaml schema, in the mean while remove the
> > "simple-bus" compatible from the binding to make sure ifc device probes
> > before any of the child devices.  Update the driver and existing DTSes
> > accordingly.
> >
> > DTS changes should be merged together with the driver/binding changes
> > if DTS maintainer is ok with it or after the driver changes are applied.
> >
>
> It's discouraged to merge DTS along with drivers (e.g. soc folks don't
> accept such pull requests), so I propose to apply it in the next cycle.

Ok.  Will separate the DTS changes in the next version.

>
> Best regards,
> Krzysztof