RE: [PATCH V3] remoteproc: virtio: Fix wdg cannot recovery remote processor
Kindly Ping...
On Sunday, December 17, 2023 1:37 PM, Joakim Zhang wrote:
> Recovery remote processor failed when wdg irq received:
> [0.842574] remoteproc remoteproc0: crash detected in cix-dsp-rproc: type
> watchdog
> [0.842750] remoteproc remoteproc0: handling crash #1 in cix-dsp-rproc
> [0.842824] remoteproc remoteproc0: recovering cix-dsp-rproc
> [0.843342] remoteproc remoteproc0: stopped remote processor cix-dsp-
> rproc
> [0.847901] rproc-virtio rproc-virtio.0.auto: Failed to associate buffer
> [0.847979] remoteproc remoteproc0: failed to probe subdevices for cix-
> dsp-rproc: -16
>
> The reason is that dma coherent mem would not be released when recovering
> the remote processor, due to rproc_virtio_remove() would not be called,
> where the mem released. It will fail when it try to allocate and associate
> buffer
> again.
>
> Releasing reserved memory from rproc_virtio_dev_release(), instead of
> rproc_virtio_remove().
>
> Fixes: 1d7b61c06dc3 ("remoteproc: virtio: Create platform device for the
> remoteproc_virtio")
> Signed-off-by: Joakim Zhang
> ---
> ChangeLogs:
> V1->V2:
> * the same for of_reserved_mem_device_release()
> V2->V3:
> * release reserved memory in rproc_virtio_dev_release()
> ---
> drivers/remoteproc/remoteproc_virtio.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/remoteproc/remoteproc_virtio.c
> b/drivers/remoteproc/remoteproc_virtio.c
> index 83d76915a6ad..25b66b113b69 100644
> --- a/drivers/remoteproc/remoteproc_virtio.c
> +++ b/drivers/remoteproc/remoteproc_virtio.c
> @@ -351,6 +351,9 @@ static void rproc_virtio_dev_release(struct device
> *dev)
>
> kfree(vdev);
>
> + of_reserved_mem_device_release(&rvdev->pdev->dev);
> + dma_release_coherent_memory(&rvdev->pdev->dev);
> +
> put_device(&rvdev->pdev->dev);
> }
>
> @@ -584,9 +587,6 @@ static void rproc_virtio_remove(struct
> platform_device *pdev)
> rproc_remove_subdev(rproc, &rvdev->subdev);
> rproc_remove_rvdev(rvdev);
>
> - of_reserved_mem_device_release(&pdev->dev);
> - dma_release_coherent_memory(&pdev->dev);
> -
> put_device(&rproc->dev);
> }
>
> --
> 2.25.1
[PATCH V3] remoteproc: virtio: Fix wdg cannot recovery remote processor
From: Joakim Zhang
Recovery remote processor failed when wdg irq received:
[0.842574] remoteproc remoteproc0: crash detected in cix-dsp-rproc: type
watchdog
[0.842750] remoteproc remoteproc0: handling crash #1 in cix-dsp-rproc
[0.842824] remoteproc remoteproc0: recovering cix-dsp-rproc
[0.843342] remoteproc remoteproc0: stopped remote processor cix-dsp-rproc
[0.847901] rproc-virtio rproc-virtio.0.auto: Failed to associate buffer
[0.847979] remoteproc remoteproc0: failed to probe subdevices for
cix-dsp-rproc: -16
The reason is that dma coherent mem would not be released when
recovering the remote processor, due to rproc_virtio_remove()
would not be called, where the mem released. It will fail when
it try to allocate and associate buffer again.
Releasing reserved memory from rproc_virtio_dev_release(), instead of
rproc_virtio_remove().
Fixes: 1d7b61c06dc3 ("remoteproc: virtio: Create platform device for the
remoteproc_virtio")
Signed-off-by: Joakim Zhang
---
ChangeLogs:
V1->V2:
* the same for of_reserved_mem_device_release()
V2->V3:
* release reserved memory in rproc_virtio_dev_release()
---
drivers/remoteproc/remoteproc_virtio.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_virtio.c
b/drivers/remoteproc/remoteproc_virtio.c
index 83d76915a6ad..25b66b113b69 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -351,6 +351,9 @@ static void rproc_virtio_dev_release(struct device *dev)
kfree(vdev);
+ of_reserved_mem_device_release(&rvdev->pdev->dev);
+ dma_release_coherent_memory(&rvdev->pdev->dev);
+
put_device(&rvdev->pdev->dev);
}
@@ -584,9 +587,6 @@ static void rproc_virtio_remove(struct platform_device
*pdev)
rproc_remove_subdev(rproc, &rvdev->subdev);
rproc_remove_rvdev(rvdev);
- of_reserved_mem_device_release(&pdev->dev);
- dma_release_coherent_memory(&pdev->dev);
-
put_device(&rproc->dev);
}
--
2.25.1
RE: [PATCH V2] remoteproc: virtio: Fix wdg cannot recovery remote processor
Hello Arnaud,
> -Original Message-
> From: Arnaud POULIQUEN
> Sent: Saturday, December 16, 2023 12:56 AM
> To: Joakim Zhang ; anders...@kernel.org;
> mathieu.poir...@linaro.org
> Cc: linux-remotep...@vger.kernel.org; linux-kernel@vger.kernel.org; cix-
> kernel-upstream
> Subject: Re: [PATCH V2] remoteproc: virtio: Fix wdg cannot recovery remote
> processor
>
> Hello Joakim,
>
> On 12/15/23 15:50, joakim.zh...@cixtech.com wrote:
> > From: Joakim Zhang
> >
> > Recovery remote processor failed when wdg irq received:
> > [0.842574] remoteproc remoteproc0: crash detected in cix-dsp-rproc:
> type watchdog
> > [0.842750] remoteproc remoteproc0: handling crash #1 in cix-dsp-rproc
> > [0.842824] remoteproc remoteproc0: recovering cix-dsp-rproc
> > [0.843342] remoteproc remoteproc0: stopped remote processor cix-dsp-
> rproc
> > [0.847901] rproc-virtio rproc-virtio.0.auto: Failed to associate buffer
> > [0.847979] remoteproc remoteproc0: failed to probe subdevices for cix-
> dsp-rproc: -16
> >
> > The reason is that dma coherent mem would not be released when
> > recovering the remote processor, due to rproc_virtio_remove() would
> > not be called, where the mem released. It will fail when it try to
> > allocate and associate buffer again.
> >
> > We can see that dma coherent mem allocated from
> > rproc_add_virtio_dev(), so should release it from
> > rproc_remove_virtio_dev(). These functions should appear symmetrically:
> > -rproc_vdev_do_start()->rproc_add_virtio_dev()-
> >dma_declare_coherent_m
> > emory()
> > -rproc_vdev_do_stop()->rproc_remove_virtio_dev()-
> >dma_release_coherent
> > _memory()
> >
> > The same for of_reserved_mem_device_init_by_idx() and
> of_reserved_mem_device_release().
> >
> > Fixes: 1d7b61c06dc3 ("remoteproc: virtio: Create platform device for
> > the remoteproc_virtio")
> > Signed-off-by: Joakim Zhang
> > ---
> > ChangeLogs:
> > V1->V2:
> > * the same for of_reserved_mem_device_release()
> > ---
> > drivers/remoteproc/remoteproc_virtio.c | 7 ---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/remoteproc/remoteproc_virtio.c
> > b/drivers/remoteproc/remoteproc_virtio.c
> > index 83d76915a6ad..e877ee78740d 100644
> > --- a/drivers/remoteproc/remoteproc_virtio.c
> > +++ b/drivers/remoteproc/remoteproc_virtio.c
> > @@ -465,8 +465,12 @@ static int rproc_add_virtio_dev(struct rproc_vdev
> > *rvdev, int id) static int rproc_remove_virtio_dev(struct device
> > *dev, void *data) {
> > struct virtio_device *vdev = dev_to_virtio(dev);
> > + struct rproc_vdev *rvdev = vdev_to_rvdev(vdev);
> >
> > unregister_virtio_device(vdev);
> > + of_reserved_mem_device_release(&rvdev->pdev->dev);
> > + dma_release_coherent_memory(&rvdev->pdev->dev);
> > +
>
> At this step, the virtio device may not be released and may still be using the
> memory.
> Do you try to move this in rproc_virtio_dev_release?
Oh, yes, thanks for the hint, I tested, and it can fix the issue, I will send
v3 soon.
Joakim
> Regards,
> Arnaud
>
> > return 0;
> > }
> >
> > @@ -584,9 +588,6 @@ static void rproc_virtio_remove(struct
> platform_device *pdev)
> > rproc_remove_subdev(rproc, &rvdev->subdev);
> > rproc_remove_rvdev(rvdev);
> >
> > - of_reserved_mem_device_release(&pdev->dev);
> > - dma_release_coherent_memory(&pdev->dev);
> > -
> > put_device(&rproc->dev);
> > }
> >
> > --
> > 2.25.1
[PATCH V2] remoteproc: virtio: Fix wdg cannot recovery remote processor
From: Joakim Zhang
Recovery remote processor failed when wdg irq received:
[0.842574] remoteproc remoteproc0: crash detected in cix-dsp-rproc: type
watchdog
[0.842750] remoteproc remoteproc0: handling crash #1 in cix-dsp-rproc
[0.842824] remoteproc remoteproc0: recovering cix-dsp-rproc
[0.843342] remoteproc remoteproc0: stopped remote processor cix-dsp-rproc
[0.847901] rproc-virtio rproc-virtio.0.auto: Failed to associate buffer
[0.847979] remoteproc remoteproc0: failed to probe subdevices for
cix-dsp-rproc: -16
The reason is that dma coherent mem would not be released when
recovering the remote processor, due to rproc_virtio_remove()
would not be called, where the mem released. It will fail when
it try to allocate and associate buffer again.
We can see that dma coherent mem allocated from rproc_add_virtio_dev(),
so should release it from rproc_remove_virtio_dev(). These functions should
appear symmetrically:
-rproc_vdev_do_start()->rproc_add_virtio_dev()->dma_declare_coherent_memory()
-rproc_vdev_do_stop()->rproc_remove_virtio_dev()->dma_release_coherent_memory()
The same for of_reserved_mem_device_init_by_idx() and
of_reserved_mem_device_release().
Fixes: 1d7b61c06dc3 ("remoteproc: virtio: Create platform device for the
remoteproc_virtio")
Signed-off-by: Joakim Zhang
---
ChangeLogs:
V1->V2:
* the same for of_reserved_mem_device_release()
---
drivers/remoteproc/remoteproc_virtio.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_virtio.c
b/drivers/remoteproc/remoteproc_virtio.c
index 83d76915a6ad..e877ee78740d 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -465,8 +465,12 @@ static int rproc_add_virtio_dev(struct rproc_vdev *rvdev,
int id)
static int rproc_remove_virtio_dev(struct device *dev, void *data)
{
struct virtio_device *vdev = dev_to_virtio(dev);
+ struct rproc_vdev *rvdev = vdev_to_rvdev(vdev);
unregister_virtio_device(vdev);
+ of_reserved_mem_device_release(&rvdev->pdev->dev);
+ dma_release_coherent_memory(&rvdev->pdev->dev);
+
return 0;
}
@@ -584,9 +588,6 @@ static void rproc_virtio_remove(struct platform_device
*pdev)
rproc_remove_subdev(rproc, &rvdev->subdev);
rproc_remove_rvdev(rvdev);
- of_reserved_mem_device_release(&pdev->dev);
- dma_release_coherent_memory(&pdev->dev);
-
put_device(&rproc->dev);
}
--
2.25.1
回复: [PATCH V1] remoteproc: virtio: Fix wdg cannot recovery remote processor
Hello maintainers,
This patch may not fix it in a correct way, after applying this patch, in
rproc_add_virtio_dev():
1) If the allocate path is dma_declare_coherent_memory(), it will be freed from
dma_release_coherent_memory(), which is expected
2) If the allocate path is of_reserved_mem_device_init_by_idx(), it will still
be freed from dma_release_coherent_memory(), which is not expected
Try to fix this issue, I also introduce another patch:
https://lore.kernel.org/lkml/20231212052130.2051333-1-joakim.zh...@cixtech.com/T/
Are there any suggestions? Thanks.
Joakim
> -邮件原件-
> 发件人: Joakim Zhang
> 发送时间: 2023年12月12日 13:24
> 收件人: anders...@kernel.org; mathieu.poir...@linaro.org;
> arnaud.pouliq...@foss.st.com
> 抄送: linux-remotep...@vger.kernel.org; linux-kernel@vger.kernel.org;
> cix-kernel-upstream ; Joakim Zhang
>
> 主题: [PATCH V1] remoteproc: virtio: Fix wdg cannot recovery remote
> processor
>
> From: Joakim Zhang
>
> Recovery remote processor failed when wdg irq received:
> [0.842574] remoteproc remoteproc0: crash detected in cix-dsp-rproc:
> type watchdog
> [0.842750] remoteproc remoteproc0: handling crash #1 in cix-dsp-rproc
> [0.842824] remoteproc remoteproc0: recovering cix-dsp-rproc
> [0.843342] remoteproc remoteproc0: stopped remote processor
> cix-dsp-rproc
> [0.847901] rproc-virtio rproc-virtio.0.auto: Failed to associate buffer
> [0.847979] remoteproc remoteproc0: failed to probe subdevices for
> cix-dsp-rproc: -16
>
> The reason is that dma coherent mem would not be released when recovering
> the remote processor, due to rproc_virtio_remove() would not be called, where
> the mem released. It will fail when it try to allocate and associate buffer
> again.
>
> We can see that dma coherent mem allocated from rproc_add_virtio_dev(), so
> should release it from rproc_remove_virtio_dev(). These functions should
> appear symmetrically:
> -rproc_vdev_do_start()->rproc_add_virtio_dev()->dma_declare_coherent_mem
> ory()
> -rproc_vdev_do_stop()->rproc_remove_virtio_dev()->dma_release_coherent_m
> emory()
>
> Fixes: 1d7b61c06dc3 ("remoteproc: virtio: Create platform device for the
> remoteproc_virtio")
> Signed-off-by: Joakim Zhang
> ---
> drivers/remoteproc/remoteproc_virtio.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/remoteproc/remoteproc_virtio.c
> b/drivers/remoteproc/remoteproc_virtio.c
> index 83d76915a6ad..725b957ee226 100644
> --- a/drivers/remoteproc/remoteproc_virtio.c
> +++ b/drivers/remoteproc/remoteproc_virtio.c
> @@ -465,8 +465,12 @@ static int rproc_add_virtio_dev(struct rproc_vdev
> *rvdev, int id) static int rproc_remove_virtio_dev(struct device *dev, void
> *data) {
> struct virtio_device *vdev = dev_to_virtio(dev);
> + struct rproc_vdev *rvdev = vdev_to_rvdev(vdev);
>
> unregister_virtio_device(vdev);
> +
> + dma_release_coherent_memory(&rvdev->pdev->dev);
> +
> return 0;
> }
>
> @@ -585,7 +589,6 @@ static void rproc_virtio_remove(struct platform_device
> *pdev)
> rproc_remove_rvdev(rvdev);
>
> of_reserved_mem_device_release(&pdev->dev);
> - dma_release_coherent_memory(&pdev->dev);
>
> put_device(&rproc->dev);
> }
> --
> 2.25.1
[PATCH V1] remoteproc: virtio: Fix wdg cannot recovery remote processor
From: Joakim Zhang
Recovery remote processor failed when wdg irq received:
[0.842574] remoteproc remoteproc0: crash detected in cix-dsp-rproc: type
watchdog
[0.842750] remoteproc remoteproc0: handling crash #1 in cix-dsp-rproc
[0.842824] remoteproc remoteproc0: recovering cix-dsp-rproc
[0.843342] remoteproc remoteproc0: stopped remote processor cix-dsp-rproc
[0.847901] rproc-virtio rproc-virtio.0.auto: Failed to associate buffer
[0.847979] remoteproc remoteproc0: failed to probe subdevices for
cix-dsp-rproc: -16
The reason is that dma coherent mem would not be released when
recovering the remote processor, due to rproc_virtio_remove()
would not be called, where the mem released. It will fail when
it try to allocate and associate buffer again.
We can see that dma coherent mem allocated from rproc_add_virtio_dev(),
so should release it from rproc_remove_virtio_dev(). These functions should
appear symmetrically:
-rproc_vdev_do_start()->rproc_add_virtio_dev()->dma_declare_coherent_memory()
-rproc_vdev_do_stop()->rproc_remove_virtio_dev()->dma_release_coherent_memory()
Fixes: 1d7b61c06dc3 ("remoteproc: virtio: Create platform device for the
remoteproc_virtio")
Signed-off-by: Joakim Zhang
---
drivers/remoteproc/remoteproc_virtio.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/remoteproc/remoteproc_virtio.c
b/drivers/remoteproc/remoteproc_virtio.c
index 83d76915a6ad..725b957ee226 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -465,8 +465,12 @@ static int rproc_add_virtio_dev(struct rproc_vdev *rvdev,
int id)
static int rproc_remove_virtio_dev(struct device *dev, void *data)
{
struct virtio_device *vdev = dev_to_virtio(dev);
+ struct rproc_vdev *rvdev = vdev_to_rvdev(vdev);
unregister_virtio_device(vdev);
+
+ dma_release_coherent_memory(&rvdev->pdev->dev);
+
return 0;
}
@@ -585,7 +589,6 @@ static void rproc_virtio_remove(struct platform_device
*pdev)
rproc_remove_rvdev(rvdev);
of_reserved_mem_device_release(&pdev->dev);
- dma_release_coherent_memory(&pdev->dev);
put_device(&rproc->dev);
}
--
2.25.1
RE: Regression v5.12-rc3: net: stmmac: re-init rx buffers when mac resume back
> -Original Message- > From: Joakim Zhang > Sent: 2021年4月14日 16:07 > To: Thierry Reding > Cc: David S. Miller ; Jakub Kicinski ; > Jon Hunter ; Giuseppe Cavallaro > ; Alexandre Torgue ; > Jose Abreu ; net...@vger.kernel.org; Linux Kernel > Mailing List ; linux-tegra > > Subject: RE: Regression v5.12-rc3: net: stmmac: re-init rx buffers when mac > resume back > > > > -Original Message- > > From: Thierry Reding > > Sent: 2021年4月14日 15:41 > > To: Joakim Zhang > > Cc: David S. Miller ; Jakub Kicinski > > ; Jon Hunter ; Giuseppe > > Cavallaro ; Alexandre Torgue > > ; Jose Abreu ; > > net...@vger.kernel.org; Linux Kernel Mailing List > > ; linux-tegra > > > > Subject: Re: Regression v5.12-rc3: net: stmmac: re-init rx buffers > > when mac resume back > > > > On Wed, Apr 14, 2021 at 02:18:58AM +, Joakim Zhang wrote: > > > > > > > -Original Message- > > > > From: Thierry Reding > > > > Sent: 2021年4月14日 0:07 > > > > To: David S. Miller ; Jakub Kicinski > > > > > > > > Cc: Joakim Zhang ; Jon Hunter > > > > ; Giuseppe Cavallaro > > > > ; Alexandre Torgue > > > > ; Jose Abreu ; > > > > net...@vger.kernel.org; Linux Kernel Mailing List > > > > ; linux-tegra > > > > > > > > Subject: Re: Regression v5.12-rc3: net: stmmac: re-init rx buffers > > > > when mac resume back > > > > > > > > On Tue, Apr 13, 2021 at 12:13:01PM +, Joakim Zhang wrote: > > > > > > > > > > Hi Jon, > > > > > > > > > > > -Original Message- > > > > > > From: Jon Hunter > > > > > > Sent: 2021年4月13日 16:41 > > > > > > To: Joakim Zhang ; Giuseppe Cavallaro > > > > > > ; Alexandre Torgue > > > > > > ; Jose Abreu > > > > > > Cc: net...@vger.kernel.org; Linux Kernel Mailing List > > > > > > ; linux-tegra > > > > > > ; Jakub Kicinski > > > > > > > > > > > > Subject: Re: Regression v5.12-rc3: net: stmmac: re-init rx > > > > > > buffers when mac resume back > > > > > > > > > > > > > > > > > > On 01/04/2021 17:28, Jon Hunter wrote: > > > > > > > > > > > > > > On 31/03/2021 12:41, Joakim Zhang wrote: > > > > > > > > > > > > > > ... > > > > > > > > > > > > > >>> In answer to your question, resuming from suspend does > > > > > > >>> work on this board without your change. We have been > > > > > > >>> testing suspend/resume now on this board since Linux v5.8 > > > > > > >>> and so we have the ability to bisect such regressions. So > > > > > > >>> it is clear to me that this is the change that caused > > > > > > this, but I am not sure why. > > > > > > >> > > > > > > >> Yes, I know this issue is regression caused by my patch. I > > > > > > >> just want to > > > > > > analyze the potential reasons. Due to the code change only > > > > > > related to the page recycle and reallocate. > > > > > > >> So I guess if this page operate need IOMMU works when IOMMU > > > > > > >> is > > > > enabled. > > > > > > Could you help check if IOMMU driver resume before STMMAC? Our > > > > > > common desire is to find the root cause, right? > > > > > > > > > > > > > > > > > > > > > Yes of course that is the desire here indeed. I had assumed > > > > > > > that the suspend/resume order was good because we have never > > > > > > > seen any problems, but nonetheless it is always good to check. > > > > > > > Using ftrace I enabled tracing of the appropriate > > > > > > > suspend/resume functions and this is what I see ... > > > > > > > > > > > > > > # tracer: function > > > > > > > # > > > > > > > # entries-in-buffer/entries-written: 4/4 #P:6 > > > > > > > # > > > > > > > #_-=> irqs-off > > > > > > > #
RE: Regression v5.12-rc3: net: stmmac: re-init rx buffers when mac resume back
> -Original Message- > From: Thierry Reding > Sent: 2021年4月14日 15:41 > To: Joakim Zhang > Cc: David S. Miller ; Jakub Kicinski ; > Jon Hunter ; Giuseppe Cavallaro > ; Alexandre Torgue ; > Jose Abreu ; net...@vger.kernel.org; Linux Kernel > Mailing List ; linux-tegra > > Subject: Re: Regression v5.12-rc3: net: stmmac: re-init rx buffers when mac > resume back > > On Wed, Apr 14, 2021 at 02:18:58AM +, Joakim Zhang wrote: > > > > > -Original Message- > > > From: Thierry Reding > > > Sent: 2021年4月14日 0:07 > > > To: David S. Miller ; Jakub Kicinski > > > > > > Cc: Joakim Zhang ; Jon Hunter > > > ; Giuseppe Cavallaro ; > > > Alexandre Torgue ; Jose Abreu > > > ; net...@vger.kernel.org; Linux Kernel Mailing > > > List ; linux-tegra > > > > > > Subject: Re: Regression v5.12-rc3: net: stmmac: re-init rx buffers > > > when mac resume back > > > > > > On Tue, Apr 13, 2021 at 12:13:01PM +, Joakim Zhang wrote: > > > > > > > > Hi Jon, > > > > > > > > > -Original Message- > > > > > From: Jon Hunter > > > > > Sent: 2021年4月13日 16:41 > > > > > To: Joakim Zhang ; Giuseppe Cavallaro > > > > > ; Alexandre Torgue > > > > > ; Jose Abreu > > > > > Cc: net...@vger.kernel.org; Linux Kernel Mailing List > > > > > ; linux-tegra > > > > > ; Jakub Kicinski > > > > > Subject: Re: Regression v5.12-rc3: net: stmmac: re-init rx > > > > > buffers when mac resume back > > > > > > > > > > > > > > > On 01/04/2021 17:28, Jon Hunter wrote: > > > > > > > > > > > > On 31/03/2021 12:41, Joakim Zhang wrote: > > > > > > > > > > > > ... > > > > > > > > > > > >>> In answer to your question, resuming from suspend does work > > > > > >>> on this board without your change. We have been testing > > > > > >>> suspend/resume now on this board since Linux v5.8 and so we > > > > > >>> have the ability to bisect such regressions. So it is clear > > > > > >>> to me that this is the change that caused > > > > > this, but I am not sure why. > > > > > >> > > > > > >> Yes, I know this issue is regression caused by my patch. I > > > > > >> just want to > > > > > analyze the potential reasons. Due to the code change only > > > > > related to the page recycle and reallocate. > > > > > >> So I guess if this page operate need IOMMU works when IOMMU > > > > > >> is > > > enabled. > > > > > Could you help check if IOMMU driver resume before STMMAC? Our > > > > > common desire is to find the root cause, right? > > > > > > > > > > > > > > > > > > Yes of course that is the desire here indeed. I had assumed > > > > > > that the suspend/resume order was good because we have never > > > > > > seen any problems, but nonetheless it is always good to check. > > > > > > Using ftrace I enabled tracing of the appropriate > > > > > > suspend/resume functions and this is what I see ... > > > > > > > > > > > > # tracer: function > > > > > > # > > > > > > # entries-in-buffer/entries-written: 4/4 #P:6 > > > > > > # > > > > > > #_-=> irqs-off > > > > > > # / _=> need-resched > > > > > > # | / _---=> hardirq/softirq > > > > > > # || / _--=> preempt-depth > > > > > > # ||| / delay > > > > > > # TASK-PID CPU# TIMESTAMP > FUNCTION > > > > > > # | | | | | > > > > > > rtcwake-748 [000] ...1 536.700777: > > > > > stmmac_pltfr_suspend <-platform_pm_suspend > > > > > > rtcwake-748 [000] ...1 536.735532: > > > > > arm_smmu_pm_suspend <-platform_pm_suspend > > > > > > rtcwake-748 [000] ...1 536.757290: > > > > > arm_smmu_pm_
RE: Regression v5.12-rc3: net: stmmac: re-init rx buffers when mac resume back
> -Original Message- > From: Thierry Reding > Sent: 2021年4月14日 0:07 > To: David S. Miller ; Jakub Kicinski > Cc: Joakim Zhang ; Jon Hunter > ; Giuseppe Cavallaro ; > Alexandre Torgue ; Jose Abreu > ; net...@vger.kernel.org; Linux Kernel Mailing List > ; linux-tegra > Subject: Re: Regression v5.12-rc3: net: stmmac: re-init rx buffers when mac > resume back > > On Tue, Apr 13, 2021 at 12:13:01PM +, Joakim Zhang wrote: > > > > Hi Jon, > > > > > -Original Message----- > > > From: Jon Hunter > > > Sent: 2021年4月13日 16:41 > > > To: Joakim Zhang ; Giuseppe Cavallaro > > > ; Alexandre Torgue > > > ; Jose Abreu > > > Cc: net...@vger.kernel.org; Linux Kernel Mailing List > > > ; linux-tegra > > > ; Jakub Kicinski > > > Subject: Re: Regression v5.12-rc3: net: stmmac: re-init rx buffers > > > when mac resume back > > > > > > > > > On 01/04/2021 17:28, Jon Hunter wrote: > > > > > > > > On 31/03/2021 12:41, Joakim Zhang wrote: > > > > > > > > ... > > > > > > > >>> In answer to your question, resuming from suspend does work on > > > >>> this board without your change. We have been testing > > > >>> suspend/resume now on this board since Linux v5.8 and so we have > > > >>> the ability to bisect such regressions. So it is clear to me > > > >>> that this is the change that caused > > > this, but I am not sure why. > > > >> > > > >> Yes, I know this issue is regression caused by my patch. I just > > > >> want to > > > analyze the potential reasons. Due to the code change only related > > > to the page recycle and reallocate. > > > >> So I guess if this page operate need IOMMU works when IOMMU is > enabled. > > > Could you help check if IOMMU driver resume before STMMAC? Our > > > common desire is to find the root cause, right? > > > > > > > > > > > > Yes of course that is the desire here indeed. I had assumed that > > > > the suspend/resume order was good because we have never seen any > > > > problems, but nonetheless it is always good to check. Using ftrace > > > > I enabled tracing of the appropriate suspend/resume functions and > > > > this is what I see ... > > > > > > > > # tracer: function > > > > # > > > > # entries-in-buffer/entries-written: 4/4 #P:6 > > > > # > > > > #_-=> irqs-off > > > > # / _=> need-resched > > > > # | / _---=> hardirq/softirq > > > > # || / _--=> preempt-depth > > > > # ||| / delay > > > > # TASK-PID CPU# TIMESTAMP FUNCTION > > > > # | | | | | > > > > rtcwake-748 [000] ...1 536.700777: > > > stmmac_pltfr_suspend <-platform_pm_suspend > > > > rtcwake-748 [000] ...1 536.735532: > > > arm_smmu_pm_suspend <-platform_pm_suspend > > > > rtcwake-748 [000] ...1 536.757290: > > > arm_smmu_pm_resume <-platform_pm_resume > > > > rtcwake-748 [003] ...1 536.856771: > > > stmmac_pltfr_resume <-platform_pm_resume > > > > > > > > > > > > So I don't see any ordering issues that could be causing this. > > > > > > > > > Another thing I have found is that for our platform, if the driver > > > for the ethernet PHY (in this case broadcom PHY) is enabled, then it > > > fails to resume but if I disable the PHY in the kernel > > > configuration, then resume works. I have found that if I move the > > > reinit of the RX buffers to before the startup of the phy, then it can > > > resume > OK with the PHY enabled. > > > > > > Does the following work for you? Does your platform use a specific > > > ethernet PHY driver? > > > > I am also looking into this issue these days, we use the Realtek RTL8211FDI > PHY, driver is drivers/net/phy/realtek.c. > > > > For our EQOS MAC integrated in our SoC, Rx side logic depends on RXC clock > from PHY, so we need phylink_start before MAC. > > > > I will test below code change tomorrow to see if it can work at my side, > > s
RE: Regression v5.12-rc3: net: stmmac: re-init rx buffers when mac resume back
Hi Jon,
> -Original Message-
> From: Jon Hunter
> Sent: 2021年4月13日 16:41
> To: Joakim Zhang ; Giuseppe Cavallaro
> ; Alexandre Torgue ;
> Jose Abreu
> Cc: net...@vger.kernel.org; Linux Kernel Mailing List
> ; linux-tegra ;
> Jakub Kicinski
> Subject: Re: Regression v5.12-rc3: net: stmmac: re-init rx buffers when mac
> resume back
>
>
> On 01/04/2021 17:28, Jon Hunter wrote:
> >
> > On 31/03/2021 12:41, Joakim Zhang wrote:
> >
> > ...
> >
> >>> In answer to your question, resuming from suspend does work on this
> >>> board without your change. We have been testing suspend/resume now
> >>> on this board since Linux v5.8 and so we have the ability to bisect
> >>> such regressions. So it is clear to me that this is the change that caused
> this, but I am not sure why.
> >>
> >> Yes, I know this issue is regression caused by my patch. I just want to
> analyze the potential reasons. Due to the code change only related to the page
> recycle and reallocate.
> >> So I guess if this page operate need IOMMU works when IOMMU is enabled.
> Could you help check if IOMMU driver resume before STMMAC? Our common
> desire is to find the root cause, right?
> >
> >
> > Yes of course that is the desire here indeed. I had assumed that the
> > suspend/resume order was good because we have never seen any problems,
> > but nonetheless it is always good to check. Using ftrace I enabled
> > tracing of the appropriate suspend/resume functions and this is what I
> > see ...
> >
> > # tracer: function
> > #
> > # entries-in-buffer/entries-written: 4/4 #P:6
> > #
> > #_-=> irqs-off
> > # / _=> need-resched
> > # | / _---=> hardirq/softirq
> > # || / _--=> preempt-depth
> > # ||| / delay
> > # TASK-PID CPU# TIMESTAMP FUNCTION
> > # | | | | |
> > rtcwake-748 [000] ...1 536.700777:
> stmmac_pltfr_suspend <-platform_pm_suspend
> > rtcwake-748 [000] ...1 536.735532:
> arm_smmu_pm_suspend <-platform_pm_suspend
> > rtcwake-748 [000] ...1 536.757290:
> arm_smmu_pm_resume <-platform_pm_resume
> > rtcwake-748 [003] ...1 536.856771:
> stmmac_pltfr_resume <-platform_pm_resume
> >
> >
> > So I don't see any ordering issues that could be causing this.
>
>
> Another thing I have found is that for our platform, if the driver for the
> ethernet
> PHY (in this case broadcom PHY) is enabled, then it fails to resume but if I
> disable the PHY in the kernel configuration, then resume works. I have found
> that if I move the reinit of the RX buffers to before the startup of the phy,
> then
> it can resume OK with the PHY enabled.
>
> Does the following work for you? Does your platform use a specific ethernet
> PHY driver?
I am also looking into this issue these days, we use the Realtek RTL8211FDI
PHY, driver is drivers/net/phy/realtek.c.
For our EQOS MAC integrated in our SoC, Rx side logic depends on RXC clock from
PHY, so we need phylink_start before MAC.
I will test below code change tomorrow to see if it can work at my side, since
it is only re-init memory, need not RXC clock.
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 208cae344ffa..071d15d86dbe 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -5416,19 +5416,20 @@ int stmmac_resume(struct device *dev)
> return ret;
> }
> + rtnl_lock();
> + mutex_lock(&priv->lock);
> + stmmac_reinit_rx_buffers(priv);
> + mutex_unlock(&priv->lock);
> +
> if (!device_may_wakeup(priv->device) || !priv->plat->pmt) {
> - rtnl_lock();
> phylink_start(priv->phylink);
> /* We may have called phylink_speed_down before */
> phylink_speed_up(priv->phylink);
> - rtnl_unlock();
> }
> - rtnl_lock();
> mutex_lock(&priv->lock);
> stmmac_reset_queues_param(priv);
> - stmmac_reinit_rx_buffers(priv);
> stmmac_free_tx_skbufs(priv);
> stmmac_clear_descriptors(priv);
>
>
> It is still not clear to us why the existing call to
> stmmac_clear_descriptors() is not suffici
RE: [PATCH net-next 0/3] net: add new properties for of_get_mac_address from nvmem
Hi Jabuk, > -Original Message- > From: Jakub Kicinski > Sent: 2021年4月10日 2:44 > To: Joakim Zhang > Cc: da...@davemloft.net; robh...@kernel.org; and...@lunn.ch; > hkallwe...@gmail.com; li...@armlinux.org.uk; frowand.l...@gmail.com; > net...@vger.kernel.org; devicet...@vger.kernel.org; > linux-kernel@vger.kernel.org; dl-linux-imx > Subject: Re: [PATCH net-next 0/3] net: add new properties for > of_get_mac_address from nvmem > > On Fri, 9 Apr 2021 17:07:08 +0800 Joakim Zhang wrote: > > This patch set adds new properties for of_get_mac_address from nvmem. > > Apart from addressing Rob's (and potentially other comments to come) please > also make sure to rebase before posting. This series doesn't seem to apply to > net-next. This patch set can be applied to latest net-next branch, the top commits as below, not sure where is the issue from, sorry. 5b489fea977c (origin/master, origin/HEAD) Merge branch 'ipa-next' 927c5043459e net: ipa: add IPA v4.11 configuration data fbb763e7e736 net: ipa: add IPA v4.5 configuration data Best Regards, Joakim Zhang
RE: [PATCH net-next 1/3] dt-bindings: net: add new properties for of_get_mac_address from nvmem
Hi Rob, > -Original Message- > From: Rob Herring > Sent: 2021年4月9日 21:50 > To: Joakim Zhang > Cc: David Miller ; Jakub Kicinski ; > Andrew Lunn ; Heiner Kallweit ; > Russell King ; Frank Rowand > ; netdev ; > devicet...@vger.kernel.org; linux-kernel@vger.kernel.org; dl-linux-imx > > Subject: Re: [PATCH net-next 1/3] dt-bindings: net: add new properties for > of_get_mac_address from nvmem > > On Fri, Apr 9, 2021 at 4:07 AM Joakim Zhang > wrote: > > > > From: Fugang Duan > > > > Currently, of_get_mac_address supports NVMEM, some platforms > > What's of_get_mac_address? This is a binding patch. Don't mix Linux things in > it. Ok, will improve it. > > MAC address that read from NVMEM efuse requires to swap bytes order, > > so add new property "nvmem_macaddr_swap" to specify the behavior. If > > the MAC address is valid from NVMEM, add new property > > "nvmem-mac-address" in ethernet node. > > > > Update these two properties in the binding documentation. > > > > Signed-off-by: Fugang Duan > > Signed-off-by: Joakim Zhang > > --- > > .../bindings/net/ethernet-controller.yaml | 14 ++ > > 1 file changed, 14 insertions(+) > > > > diff --git > > a/Documentation/devicetree/bindings/net/ethernet-controller.yaml > > b/Documentation/devicetree/bindings/net/ethernet-controller.yaml > > index e8f04687a3e0..c868c295aabf 100644 > > --- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml > > +++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml > > @@ -32,6 +32,15 @@ properties: > >- minItems: 6 > > maxItems: 6 > > > > + nvmem-mac-address: > > +allOf: > > + - $ref: /schemas/types.yaml#definitions/uint8-array > > + - minItems: 6 > > +maxItems: 6 > > +description: > > + Specifies the MAC address that was read from nvmem-cells and > dynamically > > + add the property in device node; > > Why can't you use local-mac-address or mac-address? Those too can come > from some other source. I also don't understand what Andy's do for this, per his commit message "If the MAC address is valid from NVMEM, add new property "nvmem-mac-address" in ethernet node.". He said this done _DYNAMICALLY_, but I have not found where ""nvmem-mac-address" property been added, and can't find it from ethernet node. So I send out patch first to see if reviewers can give a hint, or point out this is actually incorrect. > > + > >max-frame-size: > > $ref: /schemas/types.yaml#/definitions/uint32 > > description: > > @@ -52,6 +61,11 @@ properties: > >nvmem-cell-names: > > const: mac-address > > > > + nvmem_macaddr_swap: > > +$ref: /schemas/types.yaml#/definitions/flag > > +description: > > + swap bytes order for the 6 bytes of MAC address > > So 'nvmem-mac-address' needs to be swapped or it's swapped before writing? > In any case, this belongs in the nvmem provider. MAC address read from NVMEM cell needs to be swapped. The order of bytes that NVMEM provider provided is fixed, so I think we should swap at the NVMEM consumer side. It is more reasonable and common add a property in Documentation/devicetree/bindings/nvmem/nvmem-consumer.yaml, and swap the data in nvmem_cell_read() function. Best Regards, Joakim Zhang > Rob
[PATCH net-next 1/3] dt-bindings: net: add new properties for of_get_mac_address from nvmem
From: Fugang Duan Currently, of_get_mac_address supports NVMEM, some platforms MAC address that read from NVMEM efuse requires to swap bytes order, so add new property "nvmem_macaddr_swap" to specify the behavior. If the MAC address is valid from NVMEM, add new property "nvmem-mac-address" in ethernet node. Update these two properties in the binding documentation. Signed-off-by: Fugang Duan Signed-off-by: Joakim Zhang --- .../bindings/net/ethernet-controller.yaml | 14 ++ 1 file changed, 14 insertions(+) diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml index e8f04687a3e0..c868c295aabf 100644 --- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml +++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml @@ -32,6 +32,15 @@ properties: - minItems: 6 maxItems: 6 + nvmem-mac-address: +allOf: + - $ref: /schemas/types.yaml#definitions/uint8-array + - minItems: 6 +maxItems: 6 +description: + Specifies the MAC address that was read from nvmem-cells and dynamically + add the property in device node; + max-frame-size: $ref: /schemas/types.yaml#/definitions/uint32 description: @@ -52,6 +61,11 @@ properties: nvmem-cell-names: const: mac-address + nvmem_macaddr_swap: +$ref: /schemas/types.yaml#/definitions/flag +description: + swap bytes order for the 6 bytes of MAC address + phy-connection-type: description: Specifies interface type between the Ethernet device and a physical -- 2.17.1
[PATCH net-next 3/3] of_net: add property "nvmem-mac-address" for of_get_mac_addr()
From: Fugang Duan If MAC address read from nvmem cell and it is valid mac address, .of_get_mac_addr_nvmem() add new property "nvmem-mac-address" in ethernet node. Once user call .of_get_mac_address() to get MAC address again, it can read valid MAC address from device tree in directly. Signed-off-by: Fugang Duan Signed-off-by: Joakim Zhang --- drivers/of/of_net.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/of/of_net.c b/drivers/of/of_net.c index 6e411821583e..20c3ae17f95f 100644 --- a/drivers/of/of_net.c +++ b/drivers/of/of_net.c @@ -116,6 +116,10 @@ const void *of_get_mac_address(struct device_node *np) if (addr) return addr; + addr = of_get_mac_addr(np, "nvmem-mac-address"); + if (addr) + return addr; + return of_get_mac_addr_nvmem(np); } EXPORT_SYMBOL(of_get_mac_address); -- 2.17.1
[PATCH net-next 2/3] net: ethernet: add property "nvmem_macaddr_swap" to swap macaddr bytes order
From: Fugang Duan
ethernet controller driver call .of_get_mac_address() to get
the mac address from devictree tree, if these properties are
not present, then try to read from nvmem.
For example, read MAC address from nvmem:
of_get_mac_address()
of_get_mac_addr_nvmem()
nvmem_get_mac_address()
i.MX6x/7D/8MQ/8MM platforms ethernet MAC address read from
nvmem ocotp eFuses, but it requires to swap the six bytes
order.
The patch add optional property "nvmem_macaddr_swap" to swap
macaddr bytes order.
Signed-off-by: Fugang Duan
Signed-off-by: Joakim Zhang
---
net/ethernet/eth.c | 25 -
1 file changed, 20 insertions(+), 5 deletions(-)
diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index 4106373180c6..11057671a9d6 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -534,8 +534,10 @@ EXPORT_SYMBOL(eth_platform_get_mac_address);
int nvmem_get_mac_address(struct device *dev, void *addrbuf)
{
struct nvmem_cell *cell;
- const void *mac;
+ const unsigned char *mac;
+ unsigned char macaddr[ETH_ALEN];
size_t len;
+ int i = 0;
cell = nvmem_cell_get(dev, "mac-address");
if (IS_ERR(cell))
@@ -547,14 +549,27 @@ int nvmem_get_mac_address(struct device *dev, void
*addrbuf)
if (IS_ERR(mac))
return PTR_ERR(mac);
- if (len != ETH_ALEN || !is_valid_ether_addr(mac)) {
- kfree(mac);
- return -EINVAL;
+ if (len != ETH_ALEN)
+ goto invalid_addr;
+
+ if (dev->of_node &&
+ of_property_read_bool(dev->of_node, "nvmem_macaddr_swap")) {
+ for (i = 0; i < ETH_ALEN; i++)
+ macaddr[i] = mac[ETH_ALEN - i - 1];
+ } else {
+ ether_addr_copy(macaddr, mac);
}
- ether_addr_copy(addrbuf, mac);
+ if (!is_valid_ether_addr(macaddr))
+ goto invalid_addr;
+
+ ether_addr_copy(addrbuf, macaddr);
kfree(mac);
return 0;
+
+invalid_addr:
+ kfree(mac);
+ return -EINVAL;
}
EXPORT_SYMBOL(nvmem_get_mac_address);
--
2.17.1
[PATCH net-next 0/3] net: add new properties for of_get_mac_address from nvmem
This patch set adds new properties for of_get_mac_address from nvmem. Fugang Duan (3): dt-bindings: net: add new properties for of_get_mac_address from nvmem net: ethernet: add property "nvmem_macaddr_swap" to swap macaddr bytes order of_net: add property "nvmem-mac-address" for of_get_mac_addr() .../bindings/net/ethernet-controller.yaml | 14 +++ drivers/of/of_net.c | 4 +++ net/ethernet/eth.c| 25 +++ 3 files changed, 38 insertions(+), 5 deletions(-) -- 2.17.1
RE: [PATCH net-next 0/3] net: add new properties for of_get_mac_address from nvmem
Hi, Please ignore this patch set version, I will resend it, sorry. Best Regards, Joakim Zhang > -Original Message- > From: Joakim Zhang > Sent: 2021年4月9日 16:38 > To: da...@davemloft.net; k...@kernel.org; robh...@kernel.org; > and...@lunn.ch; hkallwe...@gmail.com; li...@armlinux.org.uk; > frowand.l...@gmail.com > Cc: net...@vger.kernel.org; devicet...@vger.kernel.org; > linux-kernel@vger.kernel.org > Subject: [PATCH net-next 0/3] net: add new properties for of_get_mac_address > from nvmem > > This patch set adds new properties for of_get_mac_address from nvmem. > > Fugang Duan (3): > dt-bindings: net: add new properties for of_get_mac_address from nvmem > net: ethernet: add property "nvmem_macaddr_swap" to swap macaddr > bytes > order > of_net: add property "nvmem-mac-address" for of_get_mac_addr() > > .../bindings/net/ethernet-controller.yaml | 14 +++ > drivers/of/of_net.c | 4 +++ > net/ethernet/eth.c| 25 > +++ > 3 files changed, 38 insertions(+), 5 deletions(-) > > -- > 2.17.1
[PATCH net-next 3/3] of_net: add property "nvmem-mac-address" for of_get_mac_addr()
From: Fugang Duan If MAC address read from nvmem cell and it is valid mac address, .of_get_mac_addr_nvmem() add new property "nvmem-mac-address" in ethernet node. Once user call .of_get_mac_address() to get MAC address again, it can read valid MAC address from device tree in directly. Signed-off-by: Fugang Duan Signed-off-by: Joakim Zhang --- drivers/of/of_net.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/of/of_net.c b/drivers/of/of_net.c index 6e411821583e..20c3ae17f95f 100644 --- a/drivers/of/of_net.c +++ b/drivers/of/of_net.c @@ -116,6 +116,10 @@ const void *of_get_mac_address(struct device_node *np) if (addr) return addr; + addr = of_get_mac_addr(np, "nvmem-mac-address"); + if (addr) + return addr; + return of_get_mac_addr_nvmem(np); } EXPORT_SYMBOL(of_get_mac_address); -- 2.17.1
[PATCH net-next 2/3] net: ethernet: add property "nvmem_macaddr_swap" to swap macaddr bytes order
From: Fugang Duan
ethernet controller driver call .of_get_mac_address() to get
the mac address from devictree tree, if these properties are
not present, then try to read from nvmem.
For example, read MAC address from nvmem:
of_get_mac_address()
of_get_mac_addr_nvmem()
nvmem_get_mac_address()
i.MX6x/7D/8MQ/8MM platforms ethernet MAC address read from
nvmem ocotp eFuses, but it requires to swap the six bytes
order.
The patch add optional property "nvmem_macaddr_swap" to swap
macaddr bytes order.
Signed-off-by: Fugang Duan
Signed-off-by: Joakim Zhang
---
net/ethernet/eth.c | 25 -
1 file changed, 20 insertions(+), 5 deletions(-)
diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index 4106373180c6..11057671a9d6 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -534,8 +534,10 @@ EXPORT_SYMBOL(eth_platform_get_mac_address);
int nvmem_get_mac_address(struct device *dev, void *addrbuf)
{
struct nvmem_cell *cell;
- const void *mac;
+ const unsigned char *mac;
+ unsigned char macaddr[ETH_ALEN];
size_t len;
+ int i = 0;
cell = nvmem_cell_get(dev, "mac-address");
if (IS_ERR(cell))
@@ -547,14 +549,27 @@ int nvmem_get_mac_address(struct device *dev, void
*addrbuf)
if (IS_ERR(mac))
return PTR_ERR(mac);
- if (len != ETH_ALEN || !is_valid_ether_addr(mac)) {
- kfree(mac);
- return -EINVAL;
+ if (len != ETH_ALEN)
+ goto invalid_addr;
+
+ if (dev->of_node &&
+ of_property_read_bool(dev->of_node, "nvmem_macaddr_swap")) {
+ for (i = 0; i < ETH_ALEN; i++)
+ macaddr[i] = mac[ETH_ALEN - i - 1];
+ } else {
+ ether_addr_copy(macaddr, mac);
}
- ether_addr_copy(addrbuf, mac);
+ if (!is_valid_ether_addr(macaddr))
+ goto invalid_addr;
+
+ ether_addr_copy(addrbuf, macaddr);
kfree(mac);
return 0;
+
+invalid_addr:
+ kfree(mac);
+ return -EINVAL;
}
EXPORT_SYMBOL(nvmem_get_mac_address);
--
2.17.1
[PATCH net-next 1/3] dt-bindings: net: add new properties for of_get_mac_address from nvmem
From: Fugang Duan Currently, of_get_mac_address supports NVMEM, some platforms MAC address that read from NVMEM efuse requires to swap bytes order, so add new property "nvmem_macaddr_swap" to specify the behavior. If the MAC address is valid from NVMEM, add new property "nvmem-mac-address" in ethernet node. Update these two properties in the binding documentation. Signed-off-by: Fugang Duan Signed-off-by: Joakim Zhang --- .../bindings/net/ethernet-controller.yaml | 14 ++ 1 file changed, 14 insertions(+) diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml index e8f04687a3e0..c868c295aabf 100644 --- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml +++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml @@ -32,6 +32,15 @@ properties: - minItems: 6 maxItems: 6 + nvmem-mac-address: +allOf: + - $ref: /schemas/types.yaml#definitions/uint8-array + - minItems: 6 +maxItems: 6 +description: + Specifies the MAC address that was read from nvmem-cells and dynamically + add the property in device node; + max-frame-size: $ref: /schemas/types.yaml#/definitions/uint32 description: @@ -52,6 +61,11 @@ properties: nvmem-cell-names: const: mac-address + nvmem_macaddr_swap: +$ref: /schemas/types.yaml#/definitions/flag +description: + swap bytes order for the 6 bytes of MAC address + phy-connection-type: description: Specifies interface type between the Ethernet device and a physical -- 2.17.1
[PATCH net-next 0/3] net: add new properties for of_get_mac_address from nvmem
This patch set adds new properties for of_get_mac_address from nvmem. Fugang Duan (3): dt-bindings: net: add new properties for of_get_mac_address from nvmem net: ethernet: add property "nvmem_macaddr_swap" to swap macaddr bytes order of_net: add property "nvmem-mac-address" for of_get_mac_addr() .../bindings/net/ethernet-controller.yaml | 14 +++ drivers/of/of_net.c | 4 +++ net/ethernet/eth.c| 25 +++ 3 files changed, 38 insertions(+), 5 deletions(-) -- 2.17.1
RE: [PATCH] net: phy: fix PHY possibly unwork after MDIO bus resume back
> -Original Message- > From: Heiner Kallweit > Sent: 2021年4月7日 18:22 > To: Joakim Zhang ; christian.me...@t2data.com; > and...@lunn.ch; li...@armlinux.org.uk; da...@davemloft.net; > k...@kernel.org > Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org; dl-linux-imx > ; Florian Fainelli > Subject: Re: [PATCH] net: phy: fix PHY possibly unwork after MDIO bus resume > back > > On 07.04.2021 12:05, Joakim Zhang wrote: > > > > Hi Heiner, > > > >> -Original Message- > >> From: Joakim Zhang > >> Sent: 2021年4月7日 15:46 > >> To: Heiner Kallweit ; > >> christian.me...@t2data.com; and...@lunn.ch; li...@armlinux.org.uk; > >> da...@davemloft.net; k...@kernel.org > >> Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org; > >> dl-linux-imx ; Florian Fainelli > >> > >> Subject: RE: [PATCH] net: phy: fix PHY possibly unwork after MDIO bus > >> resume back > >> > >> > >> Hi Heiner, > >> > >>> -Original Message- > >>> From: Heiner Kallweit > >>> Sent: 2021年4月7日 15:12 > >>> To: Joakim Zhang ; > >>> christian.me...@t2data.com; and...@lunn.ch; li...@armlinux.org.uk; > >>> da...@davemloft.net; k...@kernel.org > >>> Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org; > >>> dl-linux-imx ; Florian Fainelli > >>> > >>> Subject: Re: [PATCH] net: phy: fix PHY possibly unwork after MDIO > >>> bus resume back > >>> > >>> On 07.04.2021 03:43, Joakim Zhang wrote: > >>>> > >>>> Hi Heiner, > >>>> > >>>>> -Original Message- > >>>>> From: Heiner Kallweit > >>>>> Sent: 2021年4月7日 2:22 > >>>>> To: Joakim Zhang ; > >>>>> christian.me...@t2data.com; and...@lunn.ch; li...@armlinux.org.uk; > >>>>> da...@davemloft.net; k...@kernel.org; Russell King - ARM Linux > >>>>> > >>>>> Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org; > >>>>> dl-linux-imx ; Florian Fainelli > >>>>> > >>>>> Subject: Re: [PATCH] net: phy: fix PHY possibly unwork after MDIO > >>>>> bus resume back > >>>>> > >>>>> On 06.04.2021 13:42, Heiner Kallweit wrote: > >>>>>> On 06.04.2021 12:07, Joakim Zhang wrote: > >>>>>>> > >>>>>>>> -Original Message- > >>>>>>>> From: Heiner Kallweit > >>>>>>>> Sent: 2021年4月6日 14:29 > >>>>>>>> To: Joakim Zhang ; > >>>>>>>> christian.me...@t2data.com; and...@lunn.ch; > >>>>>>>> li...@armlinux.org.uk; da...@davemloft.net; k...@kernel.org > >>>>>>>> Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org; > >>>>>>>> dl-linux-imx > >>>>>>>> Subject: Re: [PATCH] net: phy: fix PHY possibly unwork after > >>>>>>>> MDIO bus resume back > >>>>>>>> > >>>>>>>> On 06.04.2021 04:07, Joakim Zhang wrote: > >>>>>>>>> > >>>>>>>>> Hi Heiner, > >>>>>>>>> > >>>>>>>>>> -Original Message- > >>>>>>>>>> From: Heiner Kallweit > >>>>>>>>>> Sent: 2021年4月5日 20:10 > >>>>>>>>>> To: christian.me...@t2data.com; Joakim Zhang > >>>>>>>>>> ; and...@lunn.ch; > >>>>>>>>>> li...@armlinux.org.uk; da...@davemloft.net; k...@kernel.org > >>>>>>>>>> Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org; > >>>>>>>>>> dl-linux-imx > >>>>>>>>>> Subject: Re: [PATCH] net: phy: fix PHY possibly unwork after > >>>>>>>>>> MDIO bus resume back > >>>>>>>>>> > >>>>>>>>>> On 05.04.2021 10:43, Christian Melki wrote: > >>>>>>>>>>> On 4/5/21 12:48 AM, Heiner Kallweit wrote: > >>>>>>>>>>>> On 04.04.2021 16:09, Heiner Kallweit wrote: > >>>>>>>>>>>>> On 04.04.2021 12:07, Joakim Zhang wrote: > >>>>>>>>>>>&g
RE: [PATCH] net: phy: fix PHY possibly unwork after MDIO bus resume back
Hi Heiner,
> -Original Message-
> From: Joakim Zhang
> Sent: 2021年4月7日 15:46
> To: Heiner Kallweit ; christian.me...@t2data.com;
> and...@lunn.ch; li...@armlinux.org.uk; da...@davemloft.net;
> k...@kernel.org
> Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org; dl-linux-imx
> ; Florian Fainelli
> Subject: RE: [PATCH] net: phy: fix PHY possibly unwork after MDIO bus resume
> back
>
>
> Hi Heiner,
>
> > -Original Message-
> > From: Heiner Kallweit
> > Sent: 2021年4月7日 15:12
> > To: Joakim Zhang ;
> > christian.me...@t2data.com; and...@lunn.ch; li...@armlinux.org.uk;
> > da...@davemloft.net; k...@kernel.org
> > Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org; dl-linux-imx
> > ; Florian Fainelli
> > Subject: Re: [PATCH] net: phy: fix PHY possibly unwork after MDIO bus
> > resume back
> >
> > On 07.04.2021 03:43, Joakim Zhang wrote:
> > >
> > > Hi Heiner,
> > >
> > >> -Original Message-
> > >> From: Heiner Kallweit
> > >> Sent: 2021年4月7日 2:22
> > >> To: Joakim Zhang ;
> > >> christian.me...@t2data.com; and...@lunn.ch; li...@armlinux.org.uk;
> > >> da...@davemloft.net; k...@kernel.org; Russell King - ARM Linux
> > >>
> > >> Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org;
> > >> dl-linux-imx ; Florian Fainelli
> > >>
> > >> Subject: Re: [PATCH] net: phy: fix PHY possibly unwork after MDIO
> > >> bus resume back
> > >>
> > >> On 06.04.2021 13:42, Heiner Kallweit wrote:
> > >>> On 06.04.2021 12:07, Joakim Zhang wrote:
> > >>>>
> > >>>>> -Original Message-
> > >>>>> From: Heiner Kallweit
> > >>>>> Sent: 2021年4月6日 14:29
> > >>>>> To: Joakim Zhang ;
> > >>>>> christian.me...@t2data.com; and...@lunn.ch;
> > >>>>> li...@armlinux.org.uk; da...@davemloft.net; k...@kernel.org
> > >>>>> Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org;
> > >>>>> dl-linux-imx
> > >>>>> Subject: Re: [PATCH] net: phy: fix PHY possibly unwork after
> > >>>>> MDIO bus resume back
> > >>>>>
> > >>>>> On 06.04.2021 04:07, Joakim Zhang wrote:
> > >>>>>>
> > >>>>>> Hi Heiner,
> > >>>>>>
> > >>>>>>> -Original Message-
> > >>>>>>> From: Heiner Kallweit
> > >>>>>>> Sent: 2021年4月5日 20:10
> > >>>>>>> To: christian.me...@t2data.com; Joakim Zhang
> > >>>>>>> ; and...@lunn.ch;
> > >>>>>>> li...@armlinux.org.uk; da...@davemloft.net; k...@kernel.org
> > >>>>>>> Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org;
> > >>>>>>> dl-linux-imx
> > >>>>>>> Subject: Re: [PATCH] net: phy: fix PHY possibly unwork after
> > >>>>>>> MDIO bus resume back
> > >>>>>>>
> > >>>>>>> On 05.04.2021 10:43, Christian Melki wrote:
> > >>>>>>>> On 4/5/21 12:48 AM, Heiner Kallweit wrote:
> > >>>>>>>>> On 04.04.2021 16:09, Heiner Kallweit wrote:
> > >>>>>>>>>> On 04.04.2021 12:07, Joakim Zhang wrote:
> > >>>>>>>>>>> commit 4c0d2e96ba055 ("net: phy: consider that suspend2ram
> > >> may
> > >>>>>>>>>>> cut off PHY power") invokes phy_init_hw() when MDIO bus
> > >>>>>>>>>>> resume, it will soft reset PHY if PHY driver implements
> > >>>>>>>>>>> soft_reset
> > >> callback.
> > >>>>>>>>>>> commit 764d31cacfe4 ("net: phy: micrel: set soft_reset
> > >>>>>>>>>>> callback to genphy_soft_reset for KSZ8081") adds
> > >>>>>>>>>>> soft_reset for
> > >> KSZ8081.
> > >>>>>>>>>>> After these two patches, I found i.MX6UL 14x14 EVK which
> > >>>>>>>>>>> connected to KSZ8081RNB doesn't work any more when
> system
> > >>>>> resume
> > >>>>>>>>>>> back, MAC
>
RE: [PATCH] net: phy: fix PHY possibly unwork after MDIO bus resume back
Hi Heiner,
> -Original Message-
> From: Heiner Kallweit
> Sent: 2021年4月7日 15:12
> To: Joakim Zhang ; christian.me...@t2data.com;
> and...@lunn.ch; li...@armlinux.org.uk; da...@davemloft.net;
> k...@kernel.org
> Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org; dl-linux-imx
> ; Florian Fainelli
> Subject: Re: [PATCH] net: phy: fix PHY possibly unwork after MDIO bus resume
> back
>
> On 07.04.2021 03:43, Joakim Zhang wrote:
> >
> > Hi Heiner,
> >
> >> -Original Message-
> >> From: Heiner Kallweit
> >> Sent: 2021年4月7日 2:22
> >> To: Joakim Zhang ;
> >> christian.me...@t2data.com; and...@lunn.ch; li...@armlinux.org.uk;
> >> da...@davemloft.net; k...@kernel.org; Russell King - ARM Linux
> >>
> >> Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org;
> >> dl-linux-imx ; Florian Fainelli
> >>
> >> Subject: Re: [PATCH] net: phy: fix PHY possibly unwork after MDIO bus
> >> resume back
> >>
> >> On 06.04.2021 13:42, Heiner Kallweit wrote:
> >>> On 06.04.2021 12:07, Joakim Zhang wrote:
> >>>>
> >>>>> -Original Message-
> >>>>> From: Heiner Kallweit
> >>>>> Sent: 2021年4月6日 14:29
> >>>>> To: Joakim Zhang ;
> >>>>> christian.me...@t2data.com; and...@lunn.ch; li...@armlinux.org.uk;
> >>>>> da...@davemloft.net; k...@kernel.org
> >>>>> Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org;
> >>>>> dl-linux-imx
> >>>>> Subject: Re: [PATCH] net: phy: fix PHY possibly unwork after MDIO
> >>>>> bus resume back
> >>>>>
> >>>>> On 06.04.2021 04:07, Joakim Zhang wrote:
> >>>>>>
> >>>>>> Hi Heiner,
> >>>>>>
> >>>>>>> -Original Message-
> >>>>>>> From: Heiner Kallweit
> >>>>>>> Sent: 2021年4月5日 20:10
> >>>>>>> To: christian.me...@t2data.com; Joakim Zhang
> >>>>>>> ; and...@lunn.ch;
> >>>>>>> li...@armlinux.org.uk; da...@davemloft.net; k...@kernel.org
> >>>>>>> Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org;
> >>>>>>> dl-linux-imx
> >>>>>>> Subject: Re: [PATCH] net: phy: fix PHY possibly unwork after
> >>>>>>> MDIO bus resume back
> >>>>>>>
> >>>>>>> On 05.04.2021 10:43, Christian Melki wrote:
> >>>>>>>> On 4/5/21 12:48 AM, Heiner Kallweit wrote:
> >>>>>>>>> On 04.04.2021 16:09, Heiner Kallweit wrote:
> >>>>>>>>>> On 04.04.2021 12:07, Joakim Zhang wrote:
> >>>>>>>>>>> commit 4c0d2e96ba055 ("net: phy: consider that suspend2ram
> >> may
> >>>>>>>>>>> cut off PHY power") invokes phy_init_hw() when MDIO bus
> >>>>>>>>>>> resume, it will soft reset PHY if PHY driver implements
> >>>>>>>>>>> soft_reset
> >> callback.
> >>>>>>>>>>> commit 764d31cacfe4 ("net: phy: micrel: set soft_reset
> >>>>>>>>>>> callback to genphy_soft_reset for KSZ8081") adds soft_reset
> >>>>>>>>>>> for
> >> KSZ8081.
> >>>>>>>>>>> After these two patches, I found i.MX6UL 14x14 EVK which
> >>>>>>>>>>> connected to KSZ8081RNB doesn't work any more when system
> >>>>> resume
> >>>>>>>>>>> back, MAC
> >>>>>>> driver is fec_main.c.
> >>>>>>>>>>>
> >>>>>>>>>>> It's obvious that initializing PHY hardware when MDIO bus
> >>>>>>>>>>> resume back would introduce some regression when PHY
> >>>>>>>>>>> implements soft_reset. When I
> >>>>>>>>>>
> >>>>>>>>>> Why is this obvious? Please elaborate on why a soft reset
> >>>>>>>>>> should break something.
> >>>>>>>>>>
> >>>>>>>>>>> am debugging, I found PHY works fine if MAC doesn't support
> >>>>>>>>>>> suspend/resume or
RE: [PATCH] net: phy: fix PHY possibly unwork after MDIO bus resume back
Hi Heiner,
> -Original Message-
> From: Heiner Kallweit
> Sent: 2021年4月7日 2:22
> To: Joakim Zhang ; christian.me...@t2data.com;
> and...@lunn.ch; li...@armlinux.org.uk; da...@davemloft.net;
> k...@kernel.org; Russell King - ARM Linux
> Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org; dl-linux-imx
> ; Florian Fainelli
> Subject: Re: [PATCH] net: phy: fix PHY possibly unwork after MDIO bus resume
> back
>
> On 06.04.2021 13:42, Heiner Kallweit wrote:
> > On 06.04.2021 12:07, Joakim Zhang wrote:
> >>
> >>> -Original Message-
> >>> From: Heiner Kallweit
> >>> Sent: 2021年4月6日 14:29
> >>> To: Joakim Zhang ;
> >>> christian.me...@t2data.com; and...@lunn.ch; li...@armlinux.org.uk;
> >>> da...@davemloft.net; k...@kernel.org
> >>> Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org;
> >>> dl-linux-imx
> >>> Subject: Re: [PATCH] net: phy: fix PHY possibly unwork after MDIO
> >>> bus resume back
> >>>
> >>> On 06.04.2021 04:07, Joakim Zhang wrote:
> >>>>
> >>>> Hi Heiner,
> >>>>
> >>>>> -Original Message-
> >>>>> From: Heiner Kallweit
> >>>>> Sent: 2021年4月5日 20:10
> >>>>> To: christian.me...@t2data.com; Joakim Zhang
> >>>>> ; and...@lunn.ch; li...@armlinux.org.uk;
> >>>>> da...@davemloft.net; k...@kernel.org
> >>>>> Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org;
> >>>>> dl-linux-imx
> >>>>> Subject: Re: [PATCH] net: phy: fix PHY possibly unwork after MDIO
> >>>>> bus resume back
> >>>>>
> >>>>> On 05.04.2021 10:43, Christian Melki wrote:
> >>>>>> On 4/5/21 12:48 AM, Heiner Kallweit wrote:
> >>>>>>> On 04.04.2021 16:09, Heiner Kallweit wrote:
> >>>>>>>> On 04.04.2021 12:07, Joakim Zhang wrote:
> >>>>>>>>> commit 4c0d2e96ba055 ("net: phy: consider that suspend2ram
> may
> >>>>>>>>> cut off PHY power") invokes phy_init_hw() when MDIO bus
> >>>>>>>>> resume, it will soft reset PHY if PHY driver implements soft_reset
> callback.
> >>>>>>>>> commit 764d31cacfe4 ("net: phy: micrel: set soft_reset
> >>>>>>>>> callback to genphy_soft_reset for KSZ8081") adds soft_reset for
> KSZ8081.
> >>>>>>>>> After these two patches, I found i.MX6UL 14x14 EVK which
> >>>>>>>>> connected to KSZ8081RNB doesn't work any more when system
> >>> resume
> >>>>>>>>> back, MAC
> >>>>> driver is fec_main.c.
> >>>>>>>>>
> >>>>>>>>> It's obvious that initializing PHY hardware when MDIO bus
> >>>>>>>>> resume back would introduce some regression when PHY
> >>>>>>>>> implements soft_reset. When I
> >>>>>>>>
> >>>>>>>> Why is this obvious? Please elaborate on why a soft reset
> >>>>>>>> should break something.
> >>>>>>>>
> >>>>>>>>> am debugging, I found PHY works fine if MAC doesn't support
> >>>>>>>>> suspend/resume or phy_stop()/phy_start() doesn't been called
> >>>>>>>>> during suspend/resume. This let me realize, PHY state machine
> >>>>>>>>> phy_state_machine() could do something breaks the PHY.
> >>>>>>>>>
> >>>>>>>>> As we known, MAC resume first and then MDIO bus resume when
> >>>>>>>>> system resume back from suspend. When MAC resume, usually it
> >>>>>>>>> will invoke
> >>>>>>>>> phy_start() where to change PHY state to PHY_UP, then trigger
> >>>>>>>>> the
> >>>>>>>>> stat> machine to run now. In phy_state_machine(), it will
> >>>>>>>>> start/config auto-nego, then change PHY state to PHY_NOLINK,
> >>>>>>>>> what to next is periodically check PHY link status. When MDIO
> >>>>>>>>> bus resume, it will initialize PHY hardware, including
> >>>>>>>>> soft_reset, what would soft_reset aff
RE: [PATCH] net: phy: fix PHY possibly unwork after MDIO bus resume back
> -Original Message-
> From: Heiner Kallweit
> Sent: 2021年4月6日 14:29
> To: Joakim Zhang ; christian.me...@t2data.com;
> and...@lunn.ch; li...@armlinux.org.uk; da...@davemloft.net;
> k...@kernel.org
> Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org; dl-linux-imx
>
> Subject: Re: [PATCH] net: phy: fix PHY possibly unwork after MDIO bus resume
> back
>
> On 06.04.2021 04:07, Joakim Zhang wrote:
> >
> > Hi Heiner,
> >
> >> -Original Message-
> >> From: Heiner Kallweit
> >> Sent: 2021年4月5日 20:10
> >> To: christian.me...@t2data.com; Joakim Zhang
> >> ; and...@lunn.ch; li...@armlinux.org.uk;
> >> da...@davemloft.net; k...@kernel.org
> >> Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org;
> >> dl-linux-imx
> >> Subject: Re: [PATCH] net: phy: fix PHY possibly unwork after MDIO bus
> >> resume back
> >>
> >> On 05.04.2021 10:43, Christian Melki wrote:
> >>> On 4/5/21 12:48 AM, Heiner Kallweit wrote:
> >>>> On 04.04.2021 16:09, Heiner Kallweit wrote:
> >>>>> On 04.04.2021 12:07, Joakim Zhang wrote:
> >>>>>> commit 4c0d2e96ba055 ("net: phy: consider that suspend2ram may
> >>>>>> cut off PHY power") invokes phy_init_hw() when MDIO bus resume,
> >>>>>> it will soft reset PHY if PHY driver implements soft_reset callback.
> >>>>>> commit 764d31cacfe4 ("net: phy: micrel: set soft_reset callback
> >>>>>> to genphy_soft_reset for KSZ8081") adds soft_reset for KSZ8081.
> >>>>>> After these two patches, I found i.MX6UL 14x14 EVK which
> >>>>>> connected to KSZ8081RNB doesn't work any more when system
> resume
> >>>>>> back, MAC
> >> driver is fec_main.c.
> >>>>>>
> >>>>>> It's obvious that initializing PHY hardware when MDIO bus resume
> >>>>>> back would introduce some regression when PHY implements
> >>>>>> soft_reset. When I
> >>>>>
> >>>>> Why is this obvious? Please elaborate on why a soft reset should
> >>>>> break something.
> >>>>>
> >>>>>> am debugging, I found PHY works fine if MAC doesn't support
> >>>>>> suspend/resume or phy_stop()/phy_start() doesn't been called
> >>>>>> during suspend/resume. This let me realize, PHY state machine
> >>>>>> phy_state_machine() could do something breaks the PHY.
> >>>>>>
> >>>>>> As we known, MAC resume first and then MDIO bus resume when
> >>>>>> system resume back from suspend. When MAC resume, usually it will
> >>>>>> invoke
> >>>>>> phy_start() where to change PHY state to PHY_UP, then trigger the
> >>>>>> stat> machine to run now. In phy_state_machine(), it will
> >>>>>> start/config auto-nego, then change PHY state to PHY_NOLINK, what
> >>>>>> to next is periodically check PHY link status. When MDIO bus
> >>>>>> resume, it will initialize PHY hardware, including soft_reset,
> >>>>>> what would soft_reset affect seems various from different PHYs.
> >>>>>> For KSZ8081RNB, when it in PHY_NOLINK state and then perform a
> >>>>>> soft reset,
> >> it will never complete auto-nego.
> >>>>>
> >>>>> Why? That would need to be checked in detail. Maybe chip errata
> >>>>> documentation provides a hint.
> >>>>>
> >>>>
> >>>> The KSZ8081 spec says the following about bit BMCR_PDOWN:
> >>>>
> >>>> If software reset (Register 0.15) is used to exit power-down mode
> >>>> (Register 0.11 = 1), two software reset writes (Register 0.15 = 1)
> >>>> are required. The first write clears power-down mode; the second
> >>>> write resets the chip and re-latches the pin strapping pin values.
> >>>>
> >>>> Maybe this causes the issue you see and genphy_soft_reset() isn't
> >>>> appropriate for this PHY. Please re-test with the KSZ8081 soft
> >>>> reset following the spec comment.
> >>>>
> >>>
> >>> Interesting. Never expected that behavior.
> >>> Thanks for catching it. Skimmed through the datasheets/erratas.
> >>> This is what I found (micrel.c):
>
RE: [PATCH] net: phy: fix PHY possibly unwork after MDIO bus resume back
Hi Heiner,
> -Original Message-
> From: Heiner Kallweit
> Sent: 2021年4月5日 20:10
> To: christian.me...@t2data.com; Joakim Zhang ;
> and...@lunn.ch; li...@armlinux.org.uk; da...@davemloft.net;
> k...@kernel.org
> Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org; dl-linux-imx
>
> Subject: Re: [PATCH] net: phy: fix PHY possibly unwork after MDIO bus resume
> back
>
> On 05.04.2021 10:43, Christian Melki wrote:
> > On 4/5/21 12:48 AM, Heiner Kallweit wrote:
> >> On 04.04.2021 16:09, Heiner Kallweit wrote:
> >>> On 04.04.2021 12:07, Joakim Zhang wrote:
> >>>> commit 4c0d2e96ba055 ("net: phy: consider that suspend2ram may cut
> >>>> off PHY power") invokes phy_init_hw() when MDIO bus resume, it will
> >>>> soft reset PHY if PHY driver implements soft_reset callback.
> >>>> commit 764d31cacfe4 ("net: phy: micrel: set soft_reset callback to
> >>>> genphy_soft_reset for KSZ8081") adds soft_reset for KSZ8081. After
> >>>> these two patches, I found i.MX6UL 14x14 EVK which connected to
> >>>> KSZ8081RNB doesn't work any more when system resume back, MAC
> driver is fec_main.c.
> >>>>
> >>>> It's obvious that initializing PHY hardware when MDIO bus resume
> >>>> back would introduce some regression when PHY implements
> >>>> soft_reset. When I
> >>>
> >>> Why is this obvious? Please elaborate on why a soft reset should
> >>> break something.
> >>>
> >>>> am debugging, I found PHY works fine if MAC doesn't support
> >>>> suspend/resume or phy_stop()/phy_start() doesn't been called during
> >>>> suspend/resume. This let me realize, PHY state machine
> >>>> phy_state_machine() could do something breaks the PHY.
> >>>>
> >>>> As we known, MAC resume first and then MDIO bus resume when system
> >>>> resume back from suspend. When MAC resume, usually it will invoke
> >>>> phy_start() where to change PHY state to PHY_UP, then trigger the
> >>>> stat> machine to run now. In phy_state_machine(), it will
> >>>> start/config auto-nego, then change PHY state to PHY_NOLINK, what
> >>>> to next is periodically check PHY link status. When MDIO bus
> >>>> resume, it will initialize PHY hardware, including soft_reset, what
> >>>> would soft_reset affect seems various from different PHYs. For
> >>>> KSZ8081RNB, when it in PHY_NOLINK state and then perform a soft reset,
> it will never complete auto-nego.
> >>>
> >>> Why? That would need to be checked in detail. Maybe chip errata
> >>> documentation provides a hint.
> >>>
> >>
> >> The KSZ8081 spec says the following about bit BMCR_PDOWN:
> >>
> >> If software reset (Register 0.15) is
> >> used to exit power-down mode
> >> (Register 0.11 = 1), two software
> >> reset writes (Register 0.15 = 1) are
> >> required. The first write clears
> >> power-down mode; the second
> >> write resets the chip and re-latches
> >> the pin strapping pin values.
> >>
> >> Maybe this causes the issue you see and genphy_soft_reset() isn't
> >> appropriate for this PHY. Please re-test with the KSZ8081 soft reset
> >> following the spec comment.
> >>
> >
> > Interesting. Never expected that behavior.
> > Thanks for catching it. Skimmed through the datasheets/erratas.
> > This is what I found (micrel.c):
> >
> > 10/100:
> > 8001 - Unaffected?
> > 8021/8031 - Double reset after PDOWN.
> > 8041 - Errata. PDOWN broken. Recommended do not use. Unclear if reset
> > solves the issue since errata says no error after reset but is also
> > claiming that only toggling PDOWN (may) or power will help.
> > 8051 - Double reset after PDOWN.
> > 8061 - Double reset after PDOWN.
> > 8081 - Double reset after PDOWN.
> > 8091 - Double reset after PDOWN.
> >
> > 10/100/1000:
> > Nothing in gigabit afaics.
> >
> > Switches:
> > 8862 - Not affected?
> > 8863 - Errata. PDOWN broken. Reset will not help. Workaround exists.
> > 8864 - Not affected?
> > 8873 - Errata. PDOWN broken. Reset will not help. Workaround exists.
> > 9477 - Errata. PDOWN broken. Will randomly cause link failure on
> > adjacent links. Do not use.
> >
> > This certainly explains a lot.
> >
> >>>>
> >>>> This patch chang
RE: [PATCH] net: phy: fix PHY possibly unwork after MDIO bus resume back
Hi Charistian,
> -Original Message-
> From: Christian Melki
> Sent: 2021年4月5日 16:44
> To: Heiner Kallweit ; Joakim Zhang
> ; and...@lunn.ch; li...@armlinux.org.uk;
> da...@davemloft.net; k...@kernel.org
> Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org; dl-linux-imx
>
> Subject: Re: [PATCH] net: phy: fix PHY possibly unwork after MDIO bus resume
> back
>
> On 4/5/21 12:48 AM, Heiner Kallweit wrote:
> > On 04.04.2021 16:09, Heiner Kallweit wrote:
> >> On 04.04.2021 12:07, Joakim Zhang wrote:
> >>> commit 4c0d2e96ba055 ("net: phy: consider that suspend2ram may cut
> >>> off PHY power") invokes phy_init_hw() when MDIO bus resume, it will
> >>> soft reset PHY if PHY driver implements soft_reset callback.
> >>> commit 764d31cacfe4 ("net: phy: micrel: set soft_reset callback to
> >>> genphy_soft_reset for KSZ8081") adds soft_reset for KSZ8081. After
> >>> these two patches, I found i.MX6UL 14x14 EVK which connected to
> >>> KSZ8081RNB doesn't work any more when system resume back, MAC
> driver is fec_main.c.
> >>>
> >>> It's obvious that initializing PHY hardware when MDIO bus resume
> >>> back would introduce some regression when PHY implements soft_reset.
> >>> When I
> >>
> >> Why is this obvious? Please elaborate on why a soft reset should
> >> break something.
> >>
> >>> am debugging, I found PHY works fine if MAC doesn't support
> >>> suspend/resume or phy_stop()/phy_start() doesn't been called during
> >>> suspend/resume. This let me realize, PHY state machine
> >>> phy_state_machine() could do something breaks the PHY.
> >>>
> >>> As we known, MAC resume first and then MDIO bus resume when system
> >>> resume back from suspend. When MAC resume, usually it will invoke
> >>> phy_start() where to change PHY state to PHY_UP, then trigger the
> >>> stat> machine to run now. In phy_state_machine(), it will
> >>> start/config auto-nego, then change PHY state to PHY_NOLINK, what to
> >>> next is periodically check PHY link status. When MDIO bus resume, it
> >>> will initialize PHY hardware, including soft_reset, what would
> >>> soft_reset affect seems various from different PHYs. For KSZ8081RNB,
> >>> when it in PHY_NOLINK state and then perform a soft reset, it will never
> complete auto-nego.
> >>
> >> Why? That would need to be checked in detail. Maybe chip errata
> >> documentation provides a hint.
> >>
> >
> > The KSZ8081 spec says the following about bit BMCR_PDOWN:
> >
> > If software reset (Register 0.15) is
> > used to exit power-down mode
> > (Register 0.11 = 1), two software
> > reset writes (Register 0.15 = 1) are
> > required. The first write clears
> > power-down mode; the second
> > write resets the chip and re-latches
> > the pin strapping pin values.
> >
> > Maybe this causes the issue you see and genphy_soft_reset() isn't
> > appropriate for this PHY. Please re-test with the KSZ8081 soft reset
> > following the spec comment.
> >
>
> Interesting. Never expected that behavior.
> Thanks for catching it. Skimmed through the datasheets/erratas.
> This is what I found (micrel.c):
>
> 10/100:
> 8001 - Unaffected?
> 8021/8031 - Double reset after PDOWN.
> 8041 - Errata. PDOWN broken. Recommended do not use. Unclear if reset
> solves the issue since errata says no error after reset but is also claiming
> that
> only toggling PDOWN (may) or power will help.
> 8051 - Double reset after PDOWN.
> 8061 - Double reset after PDOWN.
> 8081 - Double reset after PDOWN.
> 8091 - Double reset after PDOWN.
>
> 10/100/1000:
> Nothing in gigabit afaics.
>
> Switches:
> 8862 - Not affected?
> 8863 - Errata. PDOWN broken. Reset will not help. Workaround exists.
> 8864 - Not affected?
> 8873 - Errata. PDOWN broken. Reset will not help. Workaround exists.
> 9477 - Errata. PDOWN broken. Will randomly cause link failure on adjacent
> links.
> Do not use.
>
> This certainly explains a lot.
Thanks for digging into it. As I discussed with you before, there is no problem
with these two fixes if I did ifdown/ifup.
Almost the same route with suspend/resume. Difference is that it will start
state machine after initializing PHY. But when
resume back, state machine is running before initializing PHY. I think the key
is to figure out what would soft reset affect
in 8081, there is no any hint in spec.
Best Regards,
Joakim Zhang
> >>
RE: [PATCH] net: phy: fix PHY possibly unwork after MDIO bus resume back
Hi Heiner,
> -Original Message-
> From: Heiner Kallweit
> Sent: 2021年4月5日 6:49
> To: Joakim Zhang ; and...@lunn.ch;
> li...@armlinux.org.uk; da...@davemloft.net; k...@kernel.org
> Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org; dl-linux-imx
> ; christian.me...@t2data.com
> Subject: Re: [PATCH] net: phy: fix PHY possibly unwork after MDIO bus resume
> back
>
> On 04.04.2021 16:09, Heiner Kallweit wrote:
> > On 04.04.2021 12:07, Joakim Zhang wrote:
> >> commit 4c0d2e96ba055 ("net: phy: consider that suspend2ram may cut
> >> off PHY power") invokes phy_init_hw() when MDIO bus resume, it will
> >> soft reset PHY if PHY driver implements soft_reset callback.
> >> commit 764d31cacfe4 ("net: phy: micrel: set soft_reset callback to
> >> genphy_soft_reset for KSZ8081") adds soft_reset for KSZ8081. After
> >> these two patches, I found i.MX6UL 14x14 EVK which connected to
> >> KSZ8081RNB doesn't work any more when system resume back, MAC driver
> is fec_main.c.
> >>
> >> It's obvious that initializing PHY hardware when MDIO bus resume back
> >> would introduce some regression when PHY implements soft_reset. When
> >> I
> >
> > Why is this obvious? Please elaborate on why a soft reset should break
> > something.
> >
> >> am debugging, I found PHY works fine if MAC doesn't support
> >> suspend/resume or phy_stop()/phy_start() doesn't been called during
> >> suspend/resume. This let me realize, PHY state machine
> >> phy_state_machine() could do something breaks the PHY.
> >>
> >> As we known, MAC resume first and then MDIO bus resume when system
> >> resume back from suspend. When MAC resume, usually it will invoke
> >> phy_start() where to change PHY state to PHY_UP, then trigger the
> >> stat> machine to run now. In phy_state_machine(), it will
> >> start/config auto-nego, then change PHY state to PHY_NOLINK, what to
> >> next is periodically check PHY link status. When MDIO bus resume, it
> >> will initialize PHY hardware, including soft_reset, what would
> >> soft_reset affect seems various from different PHYs. For KSZ8081RNB,
> >> when it in PHY_NOLINK state and then perform a soft reset, it will never
> complete auto-nego.
> >
> > Why? That would need to be checked in detail. Maybe chip errata
> > documentation provides a hint.
> >
>
> The KSZ8081 spec says the following about bit BMCR_PDOWN:
>
> If software reset (Register 0.15) is
> used to exit power-down mode
> (Register 0.11 = 1), two software
> reset writes (Register 0.15 = 1) are
> required. The first write clears
> power-down mode; the second
> write resets the chip and re-latches
> the pin strapping pin values.
>
> Maybe this causes the issue you see and genphy_soft_reset() isn't appropriate
> for this PHY. Please re-test with the KSZ8081 soft reset following the spec
> comment.
Yes, I also noticed this note in spec, and tried to soft reset twice from PHY
driver, but still can't work.
What is strange is that, ifup/ifdown can work fine, which is almost the same
route with suspend/resume,
except suspend/resume has state machine running.
>
> >>
> >> This patch changes PHY state to PHY_UP when MDIO bus resume back, it
> >> should be reasonable after PHY hardware re-initialized. Also give
> >> state machine a chance to start/config auto-nego again.
> >>
> >
> > If the MAC driver calls phy_stop() on suspend, then phydev->suspended
> > is true and mdio_bus_phy_may_suspend() returns false. As a consequence
> > phydev->suspended_by_mdio_bus is false and mdio_bus_phy_resume()
> > skips the PHY hw initialization.
> > Please also note that mdio_bus_phy_suspend() calls phy_stop_machine()
> > that sets the state to PHY_UP.
> >
>
> Forgot that MDIO bus suspend is done before MAC driver suspend.
> Therefore disregard this part for now.
OK.
Best Regards,
Joakim Zhang
> > Having said that the current argumentation isn't convincing. I'm not
> > aware of such issues on other systems, therefore it's likely that
> > something is system-dependent.
> >
> > Please check the exact call sequence on your system, maybe it provides
> > a hint.
> >
> >> Signed-off-by: Joakim Zhang
> >> ---
> >> drivers/net/phy/phy_device.c | 7 +++
> >> 1 file changed, 7 insertions(+)
> >>
> >> diff --git a/drivers/net/phy/phy_device.c
> >> b/drivers/net/phy/phy_device.c index cc38e326405a..312a6f
RE: [PATCH] net: phy: fix PHY possibly unwork after MDIO bus resume back
Hi Heiner,
Thanks for your comments.
> -Original Message-
> From: Heiner Kallweit
> Sent: 2021年4月4日 22:09
> To: Joakim Zhang ; and...@lunn.ch;
> li...@armlinux.org.uk; da...@davemloft.net; k...@kernel.org
> Cc: net...@vger.kernel.org; linux-kernel@vger.kernel.org; dl-linux-imx
> ; christian.me...@t2data.com
> Subject: Re: [PATCH] net: phy: fix PHY possibly unwork after MDIO bus resume
> back
>
> On 04.04.2021 12:07, Joakim Zhang wrote:
> > commit 4c0d2e96ba055 ("net: phy: consider that suspend2ram may cut off
> > PHY power") invokes phy_init_hw() when MDIO bus resume, it will soft
> > reset PHY if PHY driver implements soft_reset callback.
> > commit 764d31cacfe4 ("net: phy: micrel: set soft_reset callback to
> > genphy_soft_reset for KSZ8081") adds soft_reset for KSZ8081. After
> > these two patches, I found i.MX6UL 14x14 EVK which connected to
> > KSZ8081RNB doesn't work any more when system resume back, MAC driver
> is fec_main.c.
> >
> > It's obvious that initializing PHY hardware when MDIO bus resume back
> > would introduce some regression when PHY implements soft_reset. When I
>
> Why is this obvious? Please elaborate on why a soft reset should break
> something.
This obvious since only above two fixes work together can trigger this issue at
my side.
> > am debugging, I found PHY works fine if MAC doesn't support
> > suspend/resume or phy_stop()/phy_start() doesn't been called during
> > suspend/resume. This let me realize, PHY state machine
> > phy_state_machine() could do something breaks the PHY.
> >
> > As we known, MAC resume first and then MDIO bus resume when system
> > resume back from suspend. When MAC resume, usually it will invoke
> > phy_start() where to change PHY state to PHY_UP, then trigger the
> > stat> machine to run now. In phy_state_machine(), it will start/config
> > auto-nego, then change PHY state to PHY_NOLINK, what to next is
> > periodically check PHY link status. When MDIO bus resume, it will
> > initialize PHY hardware, including soft_reset, what would soft_reset
> > affect seems various from different PHYs. For KSZ8081RNB, when it in
> > PHY_NOLINK state and then perform a soft reset, it will never complete
> auto-nego.
>
> Why? That would need to be checked in detail. Maybe chip errata
> documentation provides a hint.
>
> >
> > This patch changes PHY state to PHY_UP when MDIO bus resume back, it
> > should be reasonable after PHY hardware re-initialized. Also give
> > state machine a chance to start/config auto-nego again.
> >
>
> If the MAC driver calls phy_stop() on suspend, then phydev->suspended is true
> and mdio_bus_phy_may_suspend() returns false. As a consequence
> phydev->suspended_by_mdio_bus is false and mdio_bus_phy_resume()
> skips the PHY hw initialization.
Per my debugging, MDIO bus suspended before MAC, I think it is a common
behavior,
so PHY hw initialization is always called. Does this behavior be
system-dependent?
Best Regards,
Joakim Zhang
> Please also note that mdio_bus_phy_suspend() calls phy_stop_machine() that
> sets the state to PHY_UP.
>
> Having said that the current argumentation isn't convincing. I'm not aware of
> such issues on other systems, therefore it's likely that something is
> system-dependent.
>
> Please check the exact call sequence on your system, maybe it provides a hint.
>
> > Signed-off-by: Joakim Zhang
> > ---
> > drivers/net/phy/phy_device.c | 7 +++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/net/phy/phy_device.c
> > b/drivers/net/phy/phy_device.c index cc38e326405a..312a6f662481 100644
> > --- a/drivers/net/phy/phy_device.c
> > +++ b/drivers/net/phy/phy_device.c
> > @@ -306,6 +306,13 @@ static __maybe_unused int
> mdio_bus_phy_resume(struct device *dev)
> > ret = phy_resume(phydev);
> > if (ret < 0)
> > return ret;
> > +
> > + /* PHY state could be changed to PHY_NOLINK from MAC controller
> resume
> > +* rounte with phy_start(), here change to PHY_UP after re-initializing
> > +* PHY hardware, let PHY state machine to start/config auto-nego again.
> > +*/
> > + phydev->state = PHY_UP;
> > +
> > no_resume:
> > if (phydev->attached_dev && phydev->adjust_link)
> > phy_start_machine(phydev);
> >
[PATCH] net: phy: fix PHY possibly unwork after MDIO bus resume back
commit 4c0d2e96ba055 ("net: phy: consider that suspend2ram may cut
off PHY power") invokes phy_init_hw() when MDIO bus resume, it will
soft reset PHY if PHY driver implements soft_reset callback.
commit 764d31cacfe4 ("net: phy: micrel: set soft_reset callback to
genphy_soft_reset for KSZ8081") adds soft_reset for KSZ8081. After these
two patches, I found i.MX6UL 14x14 EVK which connected to KSZ8081RNB doesn't
work any more when system resume back, MAC driver is fec_main.c.
It's obvious that initializing PHY hardware when MDIO bus resume back
would introduce some regression when PHY implements soft_reset. When I
am debugging, I found PHY works fine if MAC doesn't support suspend/resume
or phy_stop()/phy_start() doesn't been called during suspend/resume. This
let me realize, PHY state machine phy_state_machine() could do something
breaks the PHY.
As we known, MAC resume first and then MDIO bus resume when system
resume back from suspend. When MAC resume, usually it will invoke
phy_start() where to change PHY state to PHY_UP, then trigger the state
machine to run now. In phy_state_machine(), it will start/config
auto-nego, then change PHY state to PHY_NOLINK, what to next is
periodically check PHY link status. When MDIO bus resume, it will
initialize PHY hardware, including soft_reset, what would soft_reset
affect seems various from different PHYs. For KSZ8081RNB, when it in
PHY_NOLINK state and then perform a soft reset, it will never complete
auto-nego.
This patch changes PHY state to PHY_UP when MDIO bus resume back, it
should be reasonable after PHY hardware re-initialized. Also give state
machine a chance to start/config auto-nego again.
Signed-off-by: Joakim Zhang
---
drivers/net/phy/phy_device.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index cc38e326405a..312a6f662481 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -306,6 +306,13 @@ static __maybe_unused int mdio_bus_phy_resume(struct
device *dev)
ret = phy_resume(phydev);
if (ret < 0)
return ret;
+
+ /* PHY state could be changed to PHY_NOLINK from MAC controller resume
+* rounte with phy_start(), here change to PHY_UP after re-initializing
+* PHY hardware, let PHY state machine to start/config auto-nego again.
+*/
+ phydev->state = PHY_UP;
+
no_resume:
if (phydev->attached_dev && phydev->adjust_link)
phy_start_machine(phydev);
--
2.17.1
RE: Regression v5.12-rc3: net: stmmac: re-init rx buffers when mac resume back
> -Original Message- > From: Jon Hunter > Sent: 2021年3月31日 19:29 > To: Joakim Zhang ; Giuseppe Cavallaro > ; Alexandre Torgue ; > Jose Abreu > Cc: net...@vger.kernel.org; Linux Kernel Mailing List > ; linux-tegra ; > Jakub Kicinski > Subject: Re: Regression v5.12-rc3: net: stmmac: re-init rx buffers when mac > resume back > > > On 31/03/2021 12:10, Joakim Zhang wrote: > > ... > > >>>>>>>> You mean one of your boards? Does other boards with STMMAC can > >>>>>>>> work > >>>>>>> fine? > >>>>>>> > >>>>>>> We have two devices with the STMMAC and one works OK and the > >>>>>>> other > >>>>> fails. > >>>>>>> They are different generation of device and so there could be > >>>>>>> some architectural differences which is causing this to only be > >>>>>>> seen on one > >>> device. > >>>>>> It's really strange, but I also don't know what architectural > >>>>>> differences could > >>>>> affect this. Sorry. > >>> > >>> > >>> I realised that for the board which fails after this change is made, > >>> it has the IOMMU enabled. The other board does not at the moment > >>> (although work is in progress to enable). If I add > >>> 'iommu.passthrough=1' to cmdline for the failing board, then it > >>> works again. So in my case, the problem is linked to the IOMMU being > enabled. > >>> > >>> Does you platform enable the IOMMU? > >> > >> Hi Jon, > >> > >> There is no IOMMU hardware available on our boards. But why IOMMU > >> would affect it during suspend/resume, and no problem in normal mode? > > > > One more add, I saw drivers/iommu/tegra-gart.c(not sure if is this) support > suspend/resume, is it possible iommu resume back after stmmac? > > > This board is the tegra186-p2771- (Jetson TX2) and uses the > arm,mmu-500 and not the above driver. OK. > In answer to your question, resuming from suspend does work on this board > without your change. We have been testing suspend/resume now on this board > since Linux v5.8 and so we have the ability to bisect such regressions. So it > is > clear to me that this is the change that caused this, but I am not sure why. Yes, I know this issue is regression caused by my patch. I just want to analyze the potential reasons. Due to the code change only related to the page recycle and reallocate. So I guess if this page operate need IOMMU works when IOMMU is enabled. Could you help check if IOMMU driver resume before STMMAC? Our common desire is to find the root cause, right? Best Regards, Joakim Zhang > Thanks > Jon > > -- > nvpublic
RE: Regression v5.12-rc3: net: stmmac: re-init rx buffers when mac resume back
> -Original Message- > From: Joakim Zhang > Sent: 2021年3月31日 15:44 > To: Jon Hunter > Cc: net...@vger.kernel.org; Linux Kernel Mailing List > ; linux-tegra ; > Jakub Kicinski > Subject: RE: Regression v5.12-rc3: net: stmmac: re-init rx buffers when mac > resume back > > > > -Original Message- > > From: Jon Hunter > > Sent: 2021年3月30日 20:51 > > To: Joakim Zhang > > Cc: net...@vger.kernel.org; Linux Kernel Mailing List > > ; linux-tegra > > ; Jakub Kicinski > > Subject: Re: Regression v5.12-rc3: net: stmmac: re-init rx buffers > > when mac resume back > > > > > > > > On 25/03/2021 08:12, Joakim Zhang wrote: > > > > ... > > > > >>>>> You mean one of your boards? Does other boards with STMMAC can > > >>>>> work > > >>>> fine? > > >>>> > > >>>> We have two devices with the STMMAC and one works OK and the > > >>>> other > > >> fails. > > >>>> They are different generation of device and so there could be > > >>>> some architectural differences which is causing this to only be > > >>>> seen on one > > device. > > >>> It's really strange, but I also don't know what architectural > > >>> differences could > > >> affect this. Sorry. > > > > > > I realised that for the board which fails after this change is made, > > it has the IOMMU enabled. The other board does not at the moment > > (although work is in progress to enable). If I add > > 'iommu.passthrough=1' to cmdline for the failing board, then it works > > again. So in my case, the problem is linked to the IOMMU being enabled. > > > > Does you platform enable the IOMMU? > > Hi Jon, > > There is no IOMMU hardware available on our boards. But why IOMMU would > affect it during suspend/resume, and no problem in normal mode? One more add, I saw drivers/iommu/tegra-gart.c(not sure if is this) support suspend/resume, is it possible iommu resume back after stmmac? Best Regards, Joakim Zhang > Best Regards, > Joakim Zhang > > Thanks > > Jon > > > > -- > > nvpublic
RE: Regression v5.12-rc3: net: stmmac: re-init rx buffers when mac resume back
> -Original Message- > From: Jon Hunter > Sent: 2021年3月30日 20:51 > To: Joakim Zhang > Cc: net...@vger.kernel.org; Linux Kernel Mailing List > ; linux-tegra ; > Jakub Kicinski > Subject: Re: Regression v5.12-rc3: net: stmmac: re-init rx buffers when mac > resume back > > > > On 25/03/2021 08:12, Joakim Zhang wrote: > > ... > > >>>>> You mean one of your boards? Does other boards with STMMAC can > >>>>> work > >>>> fine? > >>>> > >>>> We have two devices with the STMMAC and one works OK and the other > >> fails. > >>>> They are different generation of device and so there could be some > >>>> architectural differences which is causing this to only be seen on one > device. > >>> It's really strange, but I also don't know what architectural > >>> differences could > >> affect this. Sorry. > > > I realised that for the board which fails after this change is made, it has > the > IOMMU enabled. The other board does not at the moment (although work is in > progress to enable). If I add 'iommu.passthrough=1' to cmdline for the failing > board, then it works again. So in my case, the problem is linked to the IOMMU > being enabled. > > Does you platform enable the IOMMU? Hi Jon, There is no IOMMU hardware available on our boards. But why IOMMU would affect it during suspend/resume, and no problem in normal mode? Best Regards, Joakim Zhang > Thanks > Jon > > -- > nvpublic
RE: Regression v5.12-rc3: net: stmmac: re-init rx buffers when mac resume back
> -Original Message- > From: Jon Hunter > Sent: 2021年3月25日 16:01 > To: Joakim Zhang > Cc: net...@vger.kernel.org; Linux Kernel Mailing List > ; linux-tegra ; > Jakub Kicinski > Subject: Re: Regression v5.12-rc3: net: stmmac: re-init rx buffers when mac > resume back > > > On 25/03/2021 07:53, Joakim Zhang wrote: > > > >> -Original Message- > >> From: Jon Hunter > >> Sent: 2021年3月24日 20:39 > >> To: Joakim Zhang > >> Cc: net...@vger.kernel.org; Linux Kernel Mailing List > >> ; linux-tegra > >> ; Jakub Kicinski > >> Subject: Re: Regression v5.12-rc3: net: stmmac: re-init rx buffers > >> when mac resume back > >> > >> > >> > >> On 24/03/2021 12:20, Joakim Zhang wrote: > >> > >> ... > >> > >>> Sorry for this breakage at your side. > >>> > >>> You mean one of your boards? Does other boards with STMMAC can work > >> fine? > >> > >> We have two devices with the STMMAC and one works OK and the other > fails. > >> They are different generation of device and so there could be some > >> architectural differences which is causing this to only be seen on one > >> device. > > It's really strange, but I also don't know what architectural differences > > could > affect this. Sorry. > > > Maybe caching somewhere? In other words, could there be any cache flushing > that we are missing here? Have no idea, have not account into such case. > >>> We do daily test with NFS to mount rootfs, on issue found. And I add > >>> this > >> patch at the resume patch, and on error check, this should not break > suspend. > >>> I even did the overnight stress test, there is no issue found. > >>> > >>> Could you please do more test to see where the issue happen? > >> > >> The issue occurs 100% of the time on the failing board and always on > >> the first resume from suspend. Is there any more debug I can enable > >> to track down what the problem is? > >> > > > > As commit messages described, the patch aims to re-init rx buffers > > address, since the address is not fixed, so I only can recycle and then > re-allocate all of them. The page pool is allocated once when open the net > device. > > > > Could you please debug if it fails at some functions, such as > page_pool_dev_alloc_pages() ? > > > Yes that was the first thing I tried, but no obvious failures from allocating > the > pools. > > Are you certain that the problem you are seeing, that is being fixed by this > change, is generic to all devices? The commit message states that 'descriptor > write back by DMA could exhibit unusual behavior', is this a known issue in > the > STMMAC controller? If so does this impact all versions and what is the actual > problem? Yes, I confirm this patch fix issue at my side. It should not be a generic, it can reproduce at one of our boards. To be honest, I have not found the root cause, this should be a workaround, I upstream it since I think it will not affect others which don't suffer from this. Best Regards, Joakim Zhang > Jon > > -- > nvpublic
RE: Regression v5.12-rc3: net: stmmac: re-init rx buffers when mac resume back
> -Original Message- > From: Jon Hunter > Sent: 2021年3月24日 20:39 > To: Joakim Zhang > Cc: net...@vger.kernel.org; Linux Kernel Mailing List > ; linux-tegra ; > Jakub Kicinski > Subject: Re: Regression v5.12-rc3: net: stmmac: re-init rx buffers when mac > resume back > > > > On 24/03/2021 12:20, Joakim Zhang wrote: > > ... > > > Sorry for this breakage at your side. > > > > You mean one of your boards? Does other boards with STMMAC can work > fine? > > We have two devices with the STMMAC and one works OK and the other fails. > They are different generation of device and so there could be some > architectural differences which is causing this to only be seen on one device. It's really strange, but I also don't know what architectural differences could affect this. Sorry. > > We do daily test with NFS to mount rootfs, on issue found. And I add this > patch at the resume patch, and on error check, this should not break suspend. > > I even did the overnight stress test, there is no issue found. > > > > Could you please do more test to see where the issue happen? > > The issue occurs 100% of the time on the failing board and always on the first > resume from suspend. Is there any more debug I can enable to track down > what the problem is? > As commit messages described, the patch aims to re-init rx buffers address, since the address is not fixed, so I only can recycle and then re-allocate all of them. The page pool is allocated once when open the net device. Could you please debug if it fails at some functions, such as page_pool_dev_alloc_pages() ? Best Regards, Joakim Zhang > Jon > > -- > nvpublic
RE: Regression v5.12-rc3: net: stmmac: re-init rx buffers when mac resume back
> -Original Message-
> From: Jon Hunter
> Sent: 2021年3月24日 18:51
> To: Joakim Zhang
> Cc: net...@vger.kernel.org; Linux Kernel Mailing List
> ; linux-tegra ;
> Jakub Kicinski
> Subject: Regression v5.12-rc3: net: stmmac: re-init rx buffers when mac
> resume back
>
> Hi Joakim,
>
> Starting with v5.12-rc3 I noticed that one of our boards, Tegra186 Jetson TX2,
> was not long resuming from suspend. Bisect points to commit 9c63faaa931e
> ("net: stmmac: re-init rx buffers when mac resume back") and reverting this on
> top of mainline fixes the problem.
>
> Interestingly, the board appears to partially resume from suspend and I see
> ethernet appear to resume ...
>
> dwc-eth-dwmac 249.ethernet eth0: configuring for phy/rgmii link mode
> dwmac4: Master AXI performs any burst length dwc-eth-dwmac
> 249.ethernet eth0: No Safety Features support found dwc-eth-dwmac
> 249.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx
>
> I don't see any crash, but then it hangs at some point. Please note that this
> board is using NFS for mounting the rootfs.
>
> Let me know if there is any more info I can provide or tests I can run.
Hi Jon,
Sorry for this breakage at your side.
You mean one of your boards? Does other boards with STMMAC can work fine?
We do daily test with NFS to mount rootfs, on issue found. And I add this patch
at the resume patch, and on error check, this should not break suspend.
I even did the overnight stress test, there is no issue found.
Could you please do more test to see where the issue happen?
Best Regards,
Joakim Zhang
> Thanks
> Jon
>
RE: [PATCH V1] arm64: dts: imx8mp: fix FEC can't work when attached to generic phy driver
> -Original Message-
> From: Shawn Guo
> Sent: 2021年3月18日 18:48
> To: Joakim Zhang
> Cc: robh...@kernel.org; s.ha...@pengutronix.de; feste...@gmail.com;
> ker...@pengutronix.de; dl-linux-imx ;
> devicet...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH V1] arm64: dts: imx8mp: fix FEC can't work when attached
> to generic phy driver
>
> On Thu, Mar 04, 2021 at 07:40:13PM +0800, Joakim Zhang wrote:
> > Some users report that FEC can't work on i.MX8MP EVK board, it brings
> > inconvenience. The root cause should be FEC controller attached to
> > generic phy driver, as Realtek phy driver is built as module in the
> > defconfig file (CONFIG_REALTEK_PHY=m), so it is unavailable. If
> > provide "reset-gpios" property, it will reset phy when probed, and no
> > way to re-config phy since we use the generic phy dirver, which leads
> > FEC can't work.
> >
> > There are two ways to let FEC work:
> >
> > 1. If you want to use generic phy dirver, please delete "reset-gpios"
> > property, keep power-on strapping pins configurations.
> >
> > 2. If you want to use Realtek phy driver, please buildin driver
> > (CONFIG_REALTEK_PHY=y), and had better add another two reset
> > properties:
> > reset-assert-us = <2>;
> > reset-deassert-us = <15>;
> > According to RTL8211 serials PHY datasheet, for a complete PHY reset,
> > reset pin must be asserted low for at least 10ms for internal regulator.
> > Wait for at least 72ms (for internal circuits settling time) before
> > accessing the PHY register.
> >
> > This patch selects method 1, since users may waste time to find out
> > FEC failure, in most cases, they just want to use networking to debug
> > other modules.
> >
> > Fixs: commit 9e847693c6f34 ("arm64: dts: freescale: Add i.MX8MP EVK
> > board support")
> > Signed-off-by: Joakim Zhang
> > ---
> > arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> > b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> > index 7db4273cc88b..4f5c2fb33eda 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> > @@ -97,7 +97,6 @@
> > compatible = "ethernet-phy-ieee802.3-c22";
> > reg = <1>;
> > eee-broken-1000t;
> > - reset-gpios = <&gpio4 2 GPIO_ACTIVE_LOW>;
>
> Hmm, DT is describing hardware. If board schematic says there is a reset
> GPIO, we should have it.
Hi Shawn,
Seems you prefer to 2, is it possible to buildin Realtek
PHY(CONFIG_REALTEK_PHY=y)? If not, it is going to be tricky.
Best Regards,
Joakim Zhang
> Shawn
>
> > };
> > };
> > };
> > --
> > 2.17.1
> >
RE: [PATCH V1] arm64: dts: imx8mp: fix FEC can't work when attached to generic phy driver
Kindly pinging...
Best Regards,
Joakim Zhang
> -Original Message-
> From: Joakim Zhang
> Sent: 2021年3月4日 19:40
> To: robh...@kernel.org; shawn...@kernel.org; s.ha...@pengutronix.de;
> feste...@gmail.com
> Cc: ker...@pengutronix.de; dl-linux-imx ;
> devicet...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> linux-kernel@vger.kernel.org
> Subject: [PATCH V1] arm64: dts: imx8mp: fix FEC can't work when attached to
> generic phy driver
>
> Some users report that FEC can't work on i.MX8MP EVK board, it brings
> inconvenience. The root cause should be FEC controller attached to generic phy
> driver, as Realtek phy driver is built as module in the defconfig file
> (CONFIG_REALTEK_PHY=m), so it is unavailable. If provide "reset-gpios"
> property, it will reset phy when probed, and no way to re-config phy since we
> use the generic phy dirver, which leads FEC can't work.
>
> There are two ways to let FEC work:
>
> 1. If you want to use generic phy dirver, please delete "reset-gpios"
> property, keep power-on strapping pins configurations.
>
> 2. If you want to use Realtek phy driver, please buildin driver
> (CONFIG_REALTEK_PHY=y), and had better add another two reset
> properties:
> reset-assert-us = <2>;
> reset-deassert-us = <15>;
> According to RTL8211 serials PHY datasheet, for a complete PHY reset, reset
> pin must be asserted low for at least 10ms for internal regulator.
> Wait for at least 72ms (for internal circuits settling time) before accessing
> the
> PHY register.
>
> This patch selects method 1, since users may waste time to find out FEC
> failure,
> in most cases, they just want to use networking to debug other modules.
>
> Fixs: commit 9e847693c6f34 ("arm64: dts: freescale: Add i.MX8MP EVK board
> support")
> Signed-off-by: Joakim Zhang
> ---
> arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> index 7db4273cc88b..4f5c2fb33eda 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> @@ -97,7 +97,6 @@
> compatible = "ethernet-phy-ieee802.3-c22";
> reg = <1>;
> eee-broken-1000t;
> - reset-gpios = <&gpio4 2 GPIO_ACTIVE_LOW>;
> };
> };
> };
> --
> 2.17.1
[PATCH V1] arm64: dts: imx8mp: fix FEC can't work when attached to generic phy driver
Some users report that FEC can't work on i.MX8MP EVK board, it brings
inconvenience. The root cause should be FEC controller attached to
generic phy driver, as Realtek phy driver is built as module in the
defconfig file (CONFIG_REALTEK_PHY=m), so it is unavailable. If provide
"reset-gpios" property, it will reset phy when probed, and no way to
re-config phy since we use the generic phy dirver, which leads FEC can't
work.
There are two ways to let FEC work:
1. If you want to use generic phy dirver, please delete "reset-gpios"
property, keep power-on strapping pins configurations.
2. If you want to use Realtek phy driver, please buildin driver
(CONFIG_REALTEK_PHY=y), and had better add another two reset
properties:
reset-assert-us = <2>;
reset-deassert-us = <15>;
According to RTL8211 serials PHY datasheet, for a complete PHY reset,
reset pin must be asserted low for at least 10ms for internal regulator.
Wait for at least 72ms (for internal circuits settling time) before
accessing the PHY register.
This patch selects method 1, since users may waste time to find out FEC
failure, in most cases, they just want to use networking to debug other
modules.
Fixs: commit 9e847693c6f34 ("arm64: dts: freescale: Add i.MX8MP EVK board
support")
Signed-off-by: Joakim Zhang
---
arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
index 7db4273cc88b..4f5c2fb33eda 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
@@ -97,7 +97,6 @@
compatible = "ethernet-phy-ieee802.3-c22";
reg = <1>;
eee-broken-1000t;
- reset-gpios = <&gpio4 2 GPIO_ACTIVE_LOW>;
};
};
};
--
2.17.1
RE: [PATCH net 1/1] net: stmmac: fix incorrect DMA channel intr enable setting of EQoS v4.10
> -Original Message-
> From: ramesh.bab...@intel.com
> Sent: 2021年3月3日 23:09
> To: Giuseppe Cavallaro ; Alexandre Torgue
> ; Jose Abreu ; David S .
> Miller ; Jakub Kicinski ; Maxime
> Coquelin
> Cc: net...@vger.kernel.org; linux-st...@st-md-mailman.stormreply.com;
> linux-arm-ker...@lists.infradead.org; linux-kernel@vger.kernel.org; Ong Boon
> Leong ; Voon Wei Feng
> ; Wong Vee Khee ;
> Ramesh Babu B
> Subject: [PATCH net 1/1] net: stmmac: fix incorrect DMA channel intr enable
> setting of EQoS v4.10
>
> From: Ong Boon Leong
>
> We introduce dwmac410_dma_init_channel() here for both EQoS v4.10 and
> above which use different DMA_CH(n)_Interrupt_Enable bit definitions for NIE
> and AIE.
>
> Fixes: 48863ce5940f ("stmmac: add DMA support for GMAC 4.xx")
> Signed-off-by: Ong Boon Leong
> Signed-off-by: Ramesh Babu B
Reviewed-by: Joakim Zhang
Best Regards,
Joakim Zhang
> ---
> .../net/ethernet/stmicro/stmmac/dwmac4_dma.c | 19
> ++-
> 1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> index bb29bfcd62c3..62aa0e95beb7 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
> @@ -124,6 +124,23 @@ static void dwmac4_dma_init_channel(void __iomem
> *ioaddr,
> ioaddr + DMA_CHAN_INTR_ENA(chan)); }
>
> +static void dwmac410_dma_init_channel(void __iomem *ioaddr,
> + struct stmmac_dma_cfg *dma_cfg, u32 chan)
> {
> + u32 value;
> +
> + /* common channel control register config */
> + value = readl(ioaddr + DMA_CHAN_CONTROL(chan));
> + if (dma_cfg->pblx8)
> + value = value | DMA_BUS_MODE_PBL;
> +
> + writel(value, ioaddr + DMA_CHAN_CONTROL(chan));
> +
> + /* Mask interrupts by writing to CSR7 */
> + writel(DMA_CHAN_INTR_DEFAULT_MASK_4_10,
> +ioaddr + DMA_CHAN_INTR_ENA(chan)); }
> +
> static void dwmac4_dma_init(void __iomem *ioaddr,
> struct stmmac_dma_cfg *dma_cfg, int atds) { @@
> -523,7 +540,7 @@ const struct stmmac_dma_ops dwmac4_dma_ops =
> { const struct stmmac_dma_ops dwmac410_dma_ops = {
> .reset = dwmac4_dma_reset,
> .init = dwmac4_dma_init,
> - .init_chan = dwmac4_dma_init_channel,
> + .init_chan = dwmac410_dma_init_channel,
> .init_rx_chan = dwmac4_dma_init_rx_chan,
> .init_tx_chan = dwmac4_dma_init_tx_chan,
> .axi = dwmac4_dma_axi,
> --
> 2.17.1
RE: [PATCH net v4 1/1] can: can_skb_set_owner(): fix ref counting if socket was closed before setting skb ownership
> -Original Message-
> From: Joakim Zhang
> Sent: 2021年3月1日 18:57
> To: Oleksij Rempel ; m...@pengutronix.de; David S.
> Miller ; Jakub Kicinski ; Oliver
> Hartkopp ; Robin van der Gracht
>
> Cc: Andre Naujoks ; Eric Dumazet
> ; ker...@pengutronix.de; linux-...@vger.kernel.org;
> net...@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: RE: [PATCH net v4 1/1] can: can_skb_set_owner(): fix ref counting if
> socket was closed before setting skb ownership
>
>
> > -Original Message-
> > From: Oleksij Rempel
> > Sent: 2021年2月26日 17:25
> > To: m...@pengutronix.de; David S. Miller ; Jakub
> > Kicinski ; Oliver Hartkopp ;
> > Robin van der Gracht
> > Cc: Oleksij Rempel ; Andre Naujoks
> > ; Eric Dumazet ;
> > ker...@pengutronix.de; linux-...@vger.kernel.org;
> > net...@vger.kernel.org; linux-kernel@vger.kernel.org
> > Subject: [PATCH net v4 1/1] can: can_skb_set_owner(): fix ref counting
> > if socket was closed before setting skb ownership
> >
> > There are two ref count variables controlling the free()ing of a socket:
> > - struct sock::sk_refcnt - which is changed by sock_hold()/sock_put()
> > - struct sock::sk_wmem_alloc - which accounts the memory allocated by
> > the skbs in the send path.
> >
> > In case there are still TX skbs on the fly and the socket() is closed,
> > the struct sock::sk_refcnt reaches 0. In the TX-path the CAN stack
> > clones an "echo" skb, calls sock_hold() on the original socket and
> > references it. This produces the following back trace:
> >
> > | WARNING: CPU: 0 PID: 280 at lib/refcount.c:25
> > | refcount_warn_saturate+0x114/0x134
> > | refcount_t: addition on 0; use-after-free.
> > | Modules linked in: coda_vpu(E) v4l2_jpeg(E) videobuf2_vmalloc(E)
> > imx_vdoa(E)
> > | CPU: 0 PID: 280 Comm: test_can.sh Tainted: GE
> > 5.11.0-04577-gf8ff6603c617 #203
> > | Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
> > | Backtrace:
> > | [<80bafea4>] (dump_backtrace) from [<80bb0280>]
> > | (show_stack+0x20/0x24)
> > | r7: r6:600f0113 r5: r4:81441220 [<80bb0260>]
> > | (show_stack) from [<80bb593c>] (dump_stack+0xa0/0xc8) [<80bb589c>]
> > | (dump_stack) from [<8012b268>] (__warn+0xd4/0x114) r9:0019
> > | r8:80f4a8c2 r7:83e4150c r6: r5:0009 r4:80528f90
> > | [<8012b194>] (__warn) from [<80bb09c4>]
> > | (warn_slowpath_fmt+0x88/0xc8)
> > | r9:83f26400 r8:80f4a8d1 r7:0009 r6:80528f90 r5:0019
> > | r4:80f4a8c2 [<80bb0940>] (warn_slowpath_fmt) from [<80528f90>]
> > | (refcount_warn_saturate+0x114/0x134) r8: r7:
> > | r6:82b44000 r5:834e5600 r4:83f4d540 [<80528e7c>]
> > | (refcount_warn_saturate) from [<8079a4c8>]
> > | (__refcount_add.constprop.0+0x4c/0x50)
> > | [<8079a47c>] (__refcount_add.constprop.0) from [<8079a57c>]
> > | (can_put_echo_skb+0xb0/0x13c) [<8079a4cc>] (can_put_echo_skb) from
> > | [<8079ba98>] (flexcan_start_xmit+0x1c4/0x230) r9:0010
> > | r8:83f48610
> > | r7:0fdc r6:0c08 r5:82b44000 r4:834e5600 [<8079b8d4>]
> > | (flexcan_start_xmit) from [<80969078>] (netdev_start_xmit+0x44/0x70)
> > | r9:814c0ba0 r8:80c8790c r7: r6:834e5600 r5:82b44000
> > | r4:82ab1f00 [<80969034>] (netdev_start_xmit) from [<809725a4>]
> > | (dev_hard_start_xmit+0x19c/0x318) r9:814c0ba0 r8:
> > | r7:82ab1f00
> > | r6:82b44000 r5: r4:834e5600 [<80972408>]
> > | (dev_hard_start_xmit) from [<809c6584>] (sch_direct_xmit+0xcc/0x264)
> > | r10:834e5600
> > | r9: r8: r7:82b44000 r6:82ab1f00 r5:834e5600
> > | r4:83f27400 [<809c64b8>] (sch_direct_xmit) from [<809c6c0c>]
> > | (__qdisc_run+0x4f0/0x534)
> >
> > To fix this problem, only set skb ownership to sockets which have
> > still a ref count > 0.
> >
> > Cc: Oliver Hartkopp
> > Cc: Andre Naujoks
> > Suggested-by: Eric Dumazet
> > Fixes: 0ae89beb283a ("can: add destructor for self generated skbs")
> > Signed-off-by: Oleksij Rempel
>
> I will give out a test result tomorrow when the board is available. 😊
I also met this issue in the past and this patch indeed fix it. Thanks Oleksij
Rempe.
Tested-by: Joakim Zhang
Best Regards,
Joakim Zhang
> Best Regards,
> Joakim Zhang
> > ---
> > include/linux/can/skb.h | 8 ++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linux/can/skb.h
RE: [PATCH net v4 1/1] can: can_skb_set_owner(): fix ref counting if socket was closed before setting skb ownership
> -Original Message-
> From: Oleksij Rempel
> Sent: 2021年2月26日 17:25
> To: m...@pengutronix.de; David S. Miller ; Jakub
> Kicinski ; Oliver Hartkopp ;
> Robin van der Gracht
> Cc: Oleksij Rempel ; Andre Naujoks
> ; Eric Dumazet ;
> ker...@pengutronix.de; linux-...@vger.kernel.org; net...@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Subject: [PATCH net v4 1/1] can: can_skb_set_owner(): fix ref counting if
> socket
> was closed before setting skb ownership
>
> There are two ref count variables controlling the free()ing of a socket:
> - struct sock::sk_refcnt - which is changed by sock_hold()/sock_put()
> - struct sock::sk_wmem_alloc - which accounts the memory allocated by
> the skbs in the send path.
>
> In case there are still TX skbs on the fly and the socket() is closed, the
> struct
> sock::sk_refcnt reaches 0. In the TX-path the CAN stack clones an "echo" skb,
> calls sock_hold() on the original socket and references it. This produces the
> following back trace:
>
> | WARNING: CPU: 0 PID: 280 at lib/refcount.c:25
> | refcount_warn_saturate+0x114/0x134
> | refcount_t: addition on 0; use-after-free.
> | Modules linked in: coda_vpu(E) v4l2_jpeg(E) videobuf2_vmalloc(E)
> imx_vdoa(E)
> | CPU: 0 PID: 280 Comm: test_can.sh Tainted: GE
> 5.11.0-04577-gf8ff6603c617 #203
> | Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
> | Backtrace:
> | [<80bafea4>] (dump_backtrace) from [<80bb0280>] (show_stack+0x20/0x24)
> | r7: r6:600f0113 r5: r4:81441220 [<80bb0260>]
> | (show_stack) from [<80bb593c>] (dump_stack+0xa0/0xc8) [<80bb589c>]
> | (dump_stack) from [<8012b268>] (__warn+0xd4/0x114) r9:0019
> | r8:80f4a8c2 r7:83e4150c r6: r5:0009 r4:80528f90
> | [<8012b194>] (__warn) from [<80bb09c4>] (warn_slowpath_fmt+0x88/0xc8)
> | r9:83f26400 r8:80f4a8d1 r7:0009 r6:80528f90 r5:0019
> | r4:80f4a8c2 [<80bb0940>] (warn_slowpath_fmt) from [<80528f90>]
> | (refcount_warn_saturate+0x114/0x134) r8: r7:
> | r6:82b44000 r5:834e5600 r4:83f4d540 [<80528e7c>]
> | (refcount_warn_saturate) from [<8079a4c8>]
> | (__refcount_add.constprop.0+0x4c/0x50)
> | [<8079a47c>] (__refcount_add.constprop.0) from [<8079a57c>]
> | (can_put_echo_skb+0xb0/0x13c) [<8079a4cc>] (can_put_echo_skb) from
> | [<8079ba98>] (flexcan_start_xmit+0x1c4/0x230) r9:0010 r8:83f48610
> | r7:0fdc r6:0c08 r5:82b44000 r4:834e5600 [<8079b8d4>]
> | (flexcan_start_xmit) from [<80969078>] (netdev_start_xmit+0x44/0x70)
> | r9:814c0ba0 r8:80c8790c r7: r6:834e5600 r5:82b44000
> | r4:82ab1f00 [<80969034>] (netdev_start_xmit) from [<809725a4>]
> | (dev_hard_start_xmit+0x19c/0x318) r9:814c0ba0 r8: r7:82ab1f00
> | r6:82b44000 r5: r4:834e5600 [<80972408>] (dev_hard_start_xmit)
> | from [<809c6584>] (sch_direct_xmit+0xcc/0x264) r10:834e5600
> | r9: r8: r7:82b44000 r6:82ab1f00 r5:834e5600
> | r4:83f27400 [<809c64b8>] (sch_direct_xmit) from [<809c6c0c>]
> | (__qdisc_run+0x4f0/0x534)
>
> To fix this problem, only set skb ownership to sockets which have still a ref
> count > 0.
>
> Cc: Oliver Hartkopp
> Cc: Andre Naujoks
> Suggested-by: Eric Dumazet
> Fixes: 0ae89beb283a ("can: add destructor for self generated skbs")
> Signed-off-by: Oleksij Rempel
I will give out a test result tomorrow when the board is available. 😊
Best Regards,
Joakim Zhang
> ---
> include/linux/can/skb.h | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/can/skb.h b/include/linux/can/skb.h index
> 685f34cfba20..d82018cc0d0b 100644
> --- a/include/linux/can/skb.h
> +++ b/include/linux/can/skb.h
> @@ -65,8 +65,12 @@ static inline void can_skb_reserve(struct sk_buff *skb)
>
> static inline void can_skb_set_owner(struct sk_buff *skb, struct sock *sk) {
> - if (sk) {
> - sock_hold(sk);
> + /*
> + * If the socket has already been closed by user space, the refcount may
> + * already be 0 (and the socket will be freed after the last TX skb has
> + * been freed). So only increase socket refcount if the refcount is > 0.
> + */
> + if (sk && refcount_inc_not_zero(&sk->sk_refcnt)) {
> skb->destructor = sock_efree;
> skb->sk = sk;
> }
> --
> 2.29.2
RE: [PATCH V3 0/4] tools: perf: Add JSON metrics for i.MX8M platforms
Gentle pinging... Best Regards, Joakim Zhang > -Original Message- > From: John Garry > Sent: 2021年1月27日 22:14 > To: Joakim Zhang ; w...@kernel.org; > mathieu.poir...@linaro.org; leo@linaro.org; pet...@infradead.org; > mi...@redhat.com; a...@kernel.org; mark.rutl...@arm.com; > alexander.shish...@linux.intel.com; jo...@redhat.com; > namhy...@kernel.org; shawn...@kernel.org; s.ha...@pengutronix.de > Cc: ker...@pengutronix.de; feste...@gmail.com; dl-linux-imx > ; kj...@linux.ibm.com; > linux-arm-ker...@lists.infradead.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH V3 0/4] tools: perf: Add JSON metrics for i.MX8M > platforms > > On 27/01/2021 10:57, Joakim Zhang wrote: > > Add JSON metrics for i.MX8M platforms. > > > > ChangeLogs: > > V1->V2: > > * remove board level metrics (bandwidth metrics). > > V2->V3: > > * Add the missing "ScaleUnit". > > > > Joakim Zhang (4): > >perf vendor events: Fix indentation of brackets in imx8mm metrics > >perf vendor events: Add JSON metrics for imx8mn DDR Perf > >perf vendor events: Add JSON metrics for imx8mq DDR Perf > >perf vendor events: Add JSON metrics for imx8mp DDR Perf > > For the series: > Reviewed-by: John Garry
[PATCH V3 3/4] perf vendor events: Add JSON metrics for imx8mq DDR Perf
Add JSON metrics for imx8mq DDR Perf.
Signed-off-by: Joakim Zhang
---
.../arch/arm64/freescale/imx8mq/sys/ddrc.json | 37 +++
.../arm64/freescale/imx8mq/sys/metrics.json | 18 +
2 files changed, 55 insertions(+)
create mode 100644
tools/perf/pmu-events/arch/arm64/freescale/imx8mq/sys/ddrc.json
create mode 100644
tools/perf/pmu-events/arch/arm64/freescale/imx8mq/sys/metrics.json
diff --git a/tools/perf/pmu-events/arch/arm64/freescale/imx8mq/sys/ddrc.json
b/tools/perf/pmu-events/arch/arm64/freescale/imx8mq/sys/ddrc.json
new file mode 100644
index ..c8682728ddad
--- /dev/null
+++ b/tools/perf/pmu-events/arch/arm64/freescale/imx8mq/sys/ddrc.json
@@ -0,0 +1,37 @@
+[
+ {
+ "BriefDescription": "ddr cycles event",
+ "EventCode": "0x00",
+ "EventName": "imx8mq_ddr.cycles",
+ "Unit": "imx8_ddr",
+ "Compat": "i.MX8MQ"
+ },
+ {
+ "BriefDescription": "ddr read-cycles event",
+ "EventCode": "0x2a",
+ "EventName": "imx8mq_ddr.read_cycles",
+ "Unit": "imx8_ddr",
+ "Compat": "i.MX8MQ"
+ },
+ {
+ "BriefDescription": "ddr write-cycles event",
+ "EventCode": "0x2b",
+ "EventName": "imx8mq_ddr.write_cycles",
+ "Unit": "imx8_ddr",
+ "Compat": "i.MX8MQ"
+ },
+ {
+ "BriefDescription": "ddr read event",
+ "EventCode": "0x35",
+ "EventName": "imx8mq_ddr.read",
+ "Unit": "imx8_ddr",
+ "Compat": "i.MX8MQ"
+ },
+ {
+ "BriefDescription": "ddr write event",
+ "EventCode": "0x38",
+ "EventName": "imx8mq_ddr.write",
+ "Unit": "imx8_ddr",
+ "Compat": "i.MX8MQ"
+ }
+]
diff --git a/tools/perf/pmu-events/arch/arm64/freescale/imx8mq/sys/metrics.json
b/tools/perf/pmu-events/arch/arm64/freescale/imx8mq/sys/metrics.json
new file mode 100644
index ..862c98171e0d
--- /dev/null
+++ b/tools/perf/pmu-events/arch/arm64/freescale/imx8mq/sys/metrics.json
@@ -0,0 +1,18 @@
+[
+ {
+ "BriefDescription": "bytes all masters read from ddr based on
read-cycles event",
+ "MetricName": "imx8mq_ddr_read.all",
+ "MetricExpr": "imx8mq_ddr.read_cycles * 4 * 4",
+ "ScaleUnit": "9.765625e-4KB",
+ "Unit": "imx8_ddr",
+ "Compat": "i.MX8MQ"
+ },
+ {
+ "BriefDescription": "bytes all masters write to ddr based on
write-cycles event",
+ "MetricName": "imx8mq_ddr_write.all",
+ "MetricExpr": "imx8mq_ddr.write_cycles * 4 * 4",
+ "ScaleUnit": "9.765625e-4KB",
+ "Unit": "imx8_ddr",
+ "Compat": "i.MX8MQ"
+}
+]
--
2.17.1
[PATCH V3 0/4] tools: perf: Add JSON metrics for i.MX8M platforms
Add JSON metrics for i.MX8M platforms. ChangeLogs: V1->V2: * remove board level metrics (bandwidth metrics). V2->V3: * Add the missing "ScaleUnit". Joakim Zhang (4): perf vendor events: Fix indentation of brackets in imx8mm metrics perf vendor events: Add JSON metrics for imx8mn DDR Perf perf vendor events: Add JSON metrics for imx8mq DDR Perf perf vendor events: Add JSON metrics for imx8mp DDR Perf .../arm64/freescale/imx8mm/sys/metrics.json | 4 +- .../arch/arm64/freescale/imx8mn/sys/ddrc.json | 37 ++ .../arm64/freescale/imx8mn/sys/metrics.json | 18 + .../arch/arm64/freescale/imx8mp/sys/ddrc.json | 37 ++ .../arm64/freescale/imx8mp/sys/metrics.json | 466 ++ .../arch/arm64/freescale/imx8mq/sys/ddrc.json | 37 ++ .../arm64/freescale/imx8mq/sys/metrics.json | 18 + 7 files changed, 615 insertions(+), 2 deletions(-) create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/imx8mn/sys/ddrc.json create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/imx8mn/sys/metrics.json create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/imx8mp/sys/ddrc.json create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/imx8mp/sys/metrics.json create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/imx8mq/sys/ddrc.json create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/imx8mq/sys/metrics.json -- 2.17.1
[PATCH V3 4/4] perf vendor events: Add JSON metrics for imx8mp DDR Perf
Add JSON metrics for imx8mp DDR Perf.
Signed-off-by: Joakim Zhang
---
.../arch/arm64/freescale/imx8mp/sys/ddrc.json | 37 ++
.../arm64/freescale/imx8mp/sys/metrics.json | 466 ++
2 files changed, 503 insertions(+)
create mode 100644
tools/perf/pmu-events/arch/arm64/freescale/imx8mp/sys/ddrc.json
create mode 100644
tools/perf/pmu-events/arch/arm64/freescale/imx8mp/sys/metrics.json
diff --git a/tools/perf/pmu-events/arch/arm64/freescale/imx8mp/sys/ddrc.json
b/tools/perf/pmu-events/arch/arm64/freescale/imx8mp/sys/ddrc.json
new file mode 100644
index ..f9a89efc9b24
--- /dev/null
+++ b/tools/perf/pmu-events/arch/arm64/freescale/imx8mp/sys/ddrc.json
@@ -0,0 +1,37 @@
+[
+ {
+ "BriefDescription": "ddr cycles event",
+ "EventCode": "0x00",
+ "EventName": "imx8mp_ddr.cycles",
+ "Unit": "imx8_ddr",
+ "Compat": "i.MX8MP"
+ },
+ {
+ "BriefDescription": "ddr read-cycles event",
+ "EventCode": "0x2a",
+ "EventName": "imx8mp_ddr.read_cycles",
+ "Unit": "imx8_ddr",
+ "Compat": "i.MX8MP"
+ },
+ {
+ "BriefDescription": "ddr write-cycles event",
+ "EventCode": "0x2b",
+ "EventName": "imx8mp_ddr.write_cycles",
+ "Unit": "imx8_ddr",
+ "Compat": "i.MX8MP"
+ },
+ {
+ "BriefDescription": "ddr read event",
+ "EventCode": "0x35",
+ "EventName": "imx8mp_ddr.read",
+ "Unit": "imx8_ddr",
+ "Compat": "i.MX8MP"
+ },
+ {
+ "BriefDescription": "ddr write event",
+ "EventCode": "0x38",
+ "EventName": "imx8mp_ddr.write",
+ "Unit": "imx8_ddr",
+ "Compat": "i.MX8MP"
+ }
+]
diff --git a/tools/perf/pmu-events/arch/arm64/freescale/imx8mp/sys/metrics.json
b/tools/perf/pmu-events/arch/arm64/freescale/imx8mp/sys/metrics.json
new file mode 100644
index ..8b9544424b3f
--- /dev/null
+++ b/tools/perf/pmu-events/arch/arm64/freescale/imx8mp/sys/metrics.json
@@ -0,0 +1,466 @@
+[
+ {
+ "BriefDescription": "bytes of all masters read from ddr",
+ "MetricName": "imx8mp_ddr_read.all",
+ "MetricExpr":
"imx8_ddr0@axid\\-read\\,axi_mask\\=0x\\,axi_id\\=0x@",
+ "ScaleUnit": "9.765625e-4KB",
+ "Unit": "imx8_ddr",
+ "Compat": "i.MX8MP"
+ },
+ {
+ "BriefDescription": "bytes of all masters write to ddr",
+ "MetricName": "imx8mp_ddr_write.all",
+ "MetricExpr":
"imx8_ddr0@axid\\-write\\,axi_mask\\=0x\\,axi_id\\=0x@",
+ "ScaleUnit": "9.765625e-4KB",
+ "Unit": "imx8_ddr",
+ "Compat": "i.MX8MP"
+ },
+ {
+ "BriefDescription": "bytes of a53 core read from ddr",
+ "MetricName": "imx8mp_ddr_read.a53",
+ "MetricExpr":
"imx8_ddr0@axid\\-read\\,axi_mask\\=0x\\,axi_id\\=0x@",
+ "ScaleUnit": "9.765625e-4KB",
+ "Unit": "imx8_ddr",
+ "Compat": "i.MX8MP"
+ },
+ {
+ "BriefDescription": "bytes of a53 core write to ddr",
+ "MetricName": "imx8mp_ddr_write.a53",
+ "MetricExpr":
"imx8_ddr0@axid\\-write\\,axi_mask\\=0x\\,axi_id\\=0x@",
+ "ScaleUnit": "9.765625e-4KB",
+ "Unit": "imx8_ddr",
+ "Compat": "i.MX8MP"
+ },
+ {
+ "BriefDescription": "bytes of supermix(m7) core read from ddr",
+ "MetricName": "imx8mp_ddr_read.supermix",
+ "MetricExpr":
"imx8_ddr0@axid\\-read\\,axi_mask\\=0x000f\\,axi_id\\=0x0020@",
+ "ScaleUnit": "9.765625e-4KB",
+ "Unit": "imx8_ddr",
+ "Compat": "i.MX8MP"
+ },
+ {
+ "BriefDescription": "bytes of supermix(m7) write to ddr",
+ "MetricName": "imx8mp_ddr_write.supermix&
[PATCH V3 2/4] perf vendor events: Add JSON metrics for imx8mn DDR Perf
Add JSON metrics for imx8mn DDR Perf.
Signed-off-by: Joakim Zhang
---
.../arch/arm64/freescale/imx8mn/sys/ddrc.json | 37 +++
.../arm64/freescale/imx8mn/sys/metrics.json | 18 +
2 files changed, 55 insertions(+)
create mode 100644
tools/perf/pmu-events/arch/arm64/freescale/imx8mn/sys/ddrc.json
create mode 100644
tools/perf/pmu-events/arch/arm64/freescale/imx8mn/sys/metrics.json
diff --git a/tools/perf/pmu-events/arch/arm64/freescale/imx8mn/sys/ddrc.json
b/tools/perf/pmu-events/arch/arm64/freescale/imx8mn/sys/ddrc.json
new file mode 100644
index ..8352e73d6d35
--- /dev/null
+++ b/tools/perf/pmu-events/arch/arm64/freescale/imx8mn/sys/ddrc.json
@@ -0,0 +1,37 @@
+[
+ {
+ "BriefDescription": "ddr cycles event",
+ "EventCode": "0x00",
+ "EventName": "imx8mn_ddr.cycles",
+ "Unit": "imx8_ddr",
+ "Compat": "i.MX8MN"
+ },
+ {
+ "BriefDescription": "ddr read-cycles event",
+ "EventCode": "0x2a",
+ "EventName": "imx8mn_ddr.read_cycles",
+ "Unit": "imx8_ddr",
+ "Compat": "i.MX8MN"
+ },
+ {
+ "BriefDescription": "ddr write-cycles event",
+ "EventCode": "0x2b",
+ "EventName": "imx8mn_ddr.write_cycles",
+ "Unit": "imx8_ddr",
+ "Compat": "i.MX8MN"
+ },
+ {
+ "BriefDescription": "ddr read event",
+ "EventCode": "0x35",
+ "EventName": "imx8mn_ddr.read",
+ "Unit": "imx8_ddr",
+ "Compat": "i.MX8MN"
+ },
+ {
+ "BriefDescription": "ddr write event",
+ "EventCode": "0x38",
+ "EventName": "imx8mn_ddr.write",
+ "Unit": "imx8_ddr",
+ "Compat": "i.MX8MN"
+ }
+]
diff --git a/tools/perf/pmu-events/arch/arm64/freescale/imx8mn/sys/metrics.json
b/tools/perf/pmu-events/arch/arm64/freescale/imx8mn/sys/metrics.json
new file mode 100644
index ..2bbba4d8ea5b
--- /dev/null
+++ b/tools/perf/pmu-events/arch/arm64/freescale/imx8mn/sys/metrics.json
@@ -0,0 +1,18 @@
+[
+ {
+ "BriefDescription": "bytes all masters read from ddr based on
read-cycles event",
+ "MetricName": "imx8mn_ddr_read.all",
+ "MetricExpr": "imx8mn_ddr.read_cycles * 4 * 2",
+ "ScaleUnit": "9.765625e-4KB",
+ "Unit": "imx8_ddr",
+ "Compat": "i.MX8MN"
+ },
+ {
+ "BriefDescription": "bytes all masters write to ddr based on
write-cycles event",
+ "MetricName": "imx8mn_ddr_write.all",
+ "MetricExpr": "imx8mn_ddr.write_cycles * 4 * 2",
+ "ScaleUnit": "9.765625e-4KB",
+ "Unit": "imx8_ddr",
+ "Compat": "i.MX8MN"
+ }
+]
--
2.17.1
[PATCH V3 1/4] perf vendor events: Fix indentation of brackets in imx8mm metrics
Fix indentation of brackets in imx8mm metrics.
Signed-off-by: Joakim Zhang
---
.../pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json
b/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json
index 8e553b67cae6..f416fa052337 100644
--- a/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json
+++ b/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json
@@ -6,7 +6,7 @@
"ScaleUnit": "9.765625e-4KB",
"Unit": "imx8_ddr",
"Compat": "i.MX8MM"
-},
+ },
{
"BriefDescription": "bytes all masters write to ddr based on
write-cycles event",
"MetricName": "imx8mm_ddr_write.all",
@@ -14,5 +14,5 @@
"ScaleUnit": "9.765625e-4KB",
"Unit": "imx8_ddr",
"Compat": "i.MX8MM"
-}
+ }
]
--
2.17.1
RE: [PATCH] perf metricgroup: Fix system PMU metrics
> -Original Message-
> From: John Garry
> Sent: 2021年1月20日 17:16
> To: Joakim Zhang ; pet...@infradead.org;
> mi...@redhat.com; a...@kernel.org; mark.rutl...@arm.com;
> alexander.shish...@linux.intel.com; jo...@redhat.com;
> namhy...@kernel.org; irog...@google.com; kj...@linux.ibm.com
> Cc: linux-kernel@vger.kernel.org; linux...@openeuler.org
> Subject: Re: [PATCH] perf metricgroup: Fix system PMU metrics
>
> On 20/01/2021 05:15, Joakim Zhang wrote:
> >
> >> -Original Message-
> >> From: John Garry
> >> Sent: 2021年1月20日 1:33
> >> To: Joakim Zhang ; pet...@infradead.org;
> >> mi...@redhat.com; a...@kernel.org; mark.rutl...@arm.com;
> >> alexander.shish...@linux.intel.com; jo...@redhat.com;
> >> namhy...@kernel.org; irog...@google.com; kj...@linux.ibm.com
> >> Cc: linux-kernel@vger.kernel.org; linux...@openeuler.org
> >> Subject: Re: [PATCH] perf metricgroup: Fix system PMU metrics
> >>
> >> On 19/01/2021 15:47, John Garry wrote:
> >>> On 19/01/2021 10:56, Joakim Zhang wrote:
> >>>> It seems have other issue compared to 5.10 kernel after switching
> >>>> to this framework, below metric can't work.
> >>>> "MetricExpr": "(( imx8_ddr0@read\\-cycles@ +
> >>>> imx8_ddr0@write\\-cycles@
> >>>> ) * 4 * 4 / duration_time) / (750 * 100 * 4 * 4)"
> >>>> After change to:
> >>>> "MetricExpr": "(( imx8mm_ddr.read_cycles + imx8mm_ddr.write_cycles
> >>>> )
> >>>> *
> >>>> 4 * 4 / duration_time) / (750 * 100 * 4 * 4)",
> >>>
> >>> It seems that any metric which includes "duration_time" is broken,
> >>> even on x86:
> >>>
> >>> john@localhost:~/acme/tools/perf> sudo ./perf stat -v -M
> >>> L1D_Cache_Fill_BW sleep 1 Using CPUID GenuineIntel-6-3D-4 metric
> >>> expr
> >>> 64 * l1d.replacement / 10 / duration_time for
> >>> L1D_Cache_Fill_BW found event duration_time found event
> >>> l1d.replacement adding {l1d.replacement}:W,duration_time
> >>> l1d.replacement -> cpu/umask=0x1,(null)=0x1e8483,event=0x51/
> >>> Segmentation fault
> >>>
> >>>
> >>> Seems to be from my commit c2337d67199 ("perf metricgroup: Fix
> >>> metrics using aliases covering multiple PMUs")
> >>>
> >>> I'll look to fix it now.
> >>>
> >>
> >> Please try this:
> >>
> >> From 2380f1ef0250e6818b3dbc7bff4a868810875e2a Mon Sep 17
> 00:00:00
> >> 2001
> >> From: John Garry
> >> Date: Tue, 19 Jan 2021 17:29:54 +
> >> Subject: [PATCH] perf metricgroup: Fix metric support for
> >> duration_time
> >>
> >> For a metric using duration_time, the strcmp() check when finding
> >> identical events in metric_events[] is broken, as it does not
> >> consider that the event pmu_name is NULL - it would be for duration_time.
> >>
> >> As such, add a NULL check here for event pmu_name.
> >>
> >> Signed-off-by: John Garry
> >>
> >> diff --git a/tools/perf/util/metricgroup.c
> >> b/tools/perf/util/metricgroup.c index ee94d3e8dd65..277adff8017f
> >> 100644
> >> --- a/tools/perf/util/metricgroup.c
> >> +++ b/tools/perf/util/metricgroup.c
> >> @@ -280,6 +280,8 @@ static struct evsel *find_evsel_group(struct
> >> evlist *perf_evlist,
> >> */
> >>if (!has_constraint &&
> >>ev->leader != metric_events[i]->leader &&
> >> + ev->leader->pmu_name &&
> >> + metric_events[i]->leader->pmu_name &&
> >>!strcmp(ev->leader->pmu_name,
> >>metric_events[i]->leader->pmu_name))
> >>break;
> >> --
> >> 2.26.2
> >>
> >>
> >
> > For this patch: Tested-by: Joakim Zhang
> >
> > Hi John, Jolsa,
> >
> > Is there any way to avoid breaking exist metric expressions? If not, it will
> always happened after metricgroup changes.
> >
>
> They are not normally broken like that. Normally we test beforehand, but these
> cases were missed here by me. However if you were testing them previously,
> then it would be expected that you had tested them again for the final
> patchset
> which was merged.
Yes, John, sorry. I have not did the fully test before, this could be avoided.
Best Regards,
Joakim Zhang
> Anyway, we can look to add metric tests for these.
>
> @Arnaldo, I will send separate formal patch for this today.
>
> Thanks,
> John
RE: [PATCH] perf metricgroup: Fix system PMU metrics
> -Original Message-
> From: John Garry
> Sent: 2021年1月20日 1:33
> To: Joakim Zhang ; pet...@infradead.org;
> mi...@redhat.com; a...@kernel.org; mark.rutl...@arm.com;
> alexander.shish...@linux.intel.com; jo...@redhat.com;
> namhy...@kernel.org; irog...@google.com; kj...@linux.ibm.com
> Cc: linux-kernel@vger.kernel.org; linux...@openeuler.org
> Subject: Re: [PATCH] perf metricgroup: Fix system PMU metrics
>
> On 19/01/2021 15:47, John Garry wrote:
> > On 19/01/2021 10:56, Joakim Zhang wrote:
> >> It seems have other issue compared to 5.10 kernel after switching to
> >> this framework, below metric can't work.
> >> "MetricExpr": "(( imx8_ddr0@read\\-cycles@ +
> >> imx8_ddr0@write\\-cycles@
> >> ) * 4 * 4 / duration_time) / (750 * 100 * 4 * 4)"
> >> After change to:
> >> "MetricExpr": "(( imx8mm_ddr.read_cycles + imx8mm_ddr.write_cycles )
> >> *
> >> 4 * 4 / duration_time) / (750 * 100 * 4 * 4)",
> >
> > It seems that any metric which includes "duration_time" is broken,
> > even on x86:
> >
> > john@localhost:~/acme/tools/perf> sudo ./perf stat -v -M
> > L1D_Cache_Fill_BW sleep 1 Using CPUID GenuineIntel-6-3D-4 metric expr
> > 64 * l1d.replacement / 10 / duration_time for
> > L1D_Cache_Fill_BW found event duration_time found event
> > l1d.replacement adding {l1d.replacement}:W,duration_time
> > l1d.replacement -> cpu/umask=0x1,(null)=0x1e8483,event=0x51/
> > Segmentation fault
> >
> >
> > Seems to be from my commit c2337d67199 ("perf metricgroup: Fix metrics
> > using aliases covering multiple PMUs")
> >
> > I'll look to fix it now.
> >
>
> Please try this:
>
> From 2380f1ef0250e6818b3dbc7bff4a868810875e2a Mon Sep 17 00:00:00
> 2001
> From: John Garry
> Date: Tue, 19 Jan 2021 17:29:54 +
> Subject: [PATCH] perf metricgroup: Fix metric support for duration_time
>
> For a metric using duration_time, the strcmp() check when finding identical
> events in metric_events[] is broken, as it does not consider that the
> event pmu_name is NULL - it would be for duration_time.
>
> As such, add a NULL check here for event pmu_name.
>
> Signed-off-by: John Garry
>
> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> index ee94d3e8dd65..277adff8017f 100644
> --- a/tools/perf/util/metricgroup.c
> +++ b/tools/perf/util/metricgroup.c
> @@ -280,6 +280,8 @@ static struct evsel *find_evsel_group(struct evlist
> *perf_evlist,
>*/
> if (!has_constraint &&
> ev->leader != metric_events[i]->leader &&
> + ev->leader->pmu_name &&
> + metric_events[i]->leader->pmu_name &&
> !strcmp(ev->leader->pmu_name,
> metric_events[i]->leader->pmu_name))
> break;
> --
> 2.26.2
>
>
For this patch: Tested-by: Joakim Zhang
Hi John, Jolsa,
Is there any way to avoid breaking exist metric expressions? If not, it will
always happened after metricgroup changes.
I recall that Jolsa mentioned it before, but I don’t remember it very clearly.
Thanks a lot for John's bug fix.
Best Regards,
Joakim Zhang
RE: [PATCH] perf metricgroup: Fix system PMU metrics
> -Original Message-
> From: John Garry
> Sent: 2021年1月19日 19:05
> To: Joakim Zhang ; pet...@infradead.org;
> mi...@redhat.com; a...@kernel.org; mark.rutl...@arm.com;
> alexander.shish...@linux.intel.com; jo...@redhat.com;
> namhy...@kernel.org; irog...@google.com; kj...@linux.ibm.com
> Cc: linux-kernel@vger.kernel.org; linux...@openeuler.org
> Subject: Re: [PATCH] perf metricgroup: Fix system PMU metrics
>
> On 19/01/2021 10:56, Joakim Zhang wrote:
> >> Joakim reports that getting "perf stat" for multiple system PMU
> >> metrics
> >> segfaults:
> >> ./perf stat -a -I 1000 -M imx8mm_ddr_write.all,imx8mm_ddr_write.all
> >> Segmentation fault
> >>
> >> While the same works without issue for a single metric.
> >>
> >> The logic in metricgroup__add_metric_sys_event_iter() is broken, in
> >> that
> >> add_metric() @m argument should be NULL for each new metric. Fix by
> >> not passing a holder for that, and rather make local in
> >> metricgroup__add_metric_sys_event_iter().
> >>
> >> Fixes: be335ec28efa ("perf metricgroup: Support adding metrics for
> >> system
> >> PMUs")
> >> Reported-by: Joakim Zhang
> >> Signed-off-by: John Garry
> > root@imx8mmevk:~# ./perf stat -a -I 1000 -M
> imx8mm_ddr_read.all,imx8mm_ddr_write
>
>.all
> > # time counts unit events
> > 1.001446500 40832
> imx8mm_ddr.read_cycles#638.0 KB imx8mm_ddr_read.all
> > 1.001446500 16973
> imx8mm_ddr.write_cycles #265.2 KB imx8mm_ddr_write.all
> > 2.003150250 28836
> imx8mm_ddr.read_cycles#450.6 KB imx8mm_ddr_read.all
> > 2.003150250 6705
> imx8mm_ddr.write_cycles #104.8 KB imx8mm_ddr_write.all
> >
> > For this issue, Tested-by: Joakim Zhang
> >
> > Hi John,
> >
> > It seems have other issue compared to 5.10 kernel after switching to this
> framework, below metric can't work.
> > "MetricExpr": "(( imx8_ddr0@read\\-cycles@ + imx8_ddr0@write\\-cycles@ )
> * 4 * 4 / duration_time) / (750 * 100 * 4 * 4)"
> > After change to:
> > "MetricExpr": "(( imx8mm_ddr.read_cycles + imx8mm_ddr.write_cycles ) *
> > 4 * 4 / duration_time) / (750 * 100 * 4 * 4)",
>
> Hmmm... not sure what you mean by "compared to 5.10 kernel". As far as I'm
> concerned, none of this was supported in 5.10 and metrics did not work for
> arm64. Support for sys PMU events+metrics only came in 5.11-rc.
Yes, 5.10 doesn't support ARM64. I add some code let it work locally. And,
"MetricExpr": "(( imx8_ddr0@read\\-cycles@ + imx8_ddr0@write\\-cycles@ ) * 4 *
4 / duration_time) / (750 * 100 * 4 * 4)"
Above metric expression can work fine.
> Anyway, can you share the full metric event which you say does not work, not
> just the "MetricExpr"?
OK, Could help check below metric? Thanks.
"MetricExpr": "(( imx8_ddr0@read\\-cycles@ + imx8_ddr0@write\\-cycles@ ) * 4 *
4 / duration_time) / (750 * 100 * 4 * 4)"
or
"MetricExpr": "(( imx8mm_ddr.read_cycles + imx8mm_ddr.write_cycles ) * 4 * 4 /
duration_time) / (750 * 100 * 4 * 4)"
Best Regards,
Joakim Zhang
> Thanks,
> John
RE: [PATCH] perf metricgroup: Fix system PMU metrics
> -Original Message-
> From: John Garry
> Sent: 2021年1月19日 18:04
> To: pet...@infradead.org; mi...@redhat.com; a...@kernel.org;
> mark.rutl...@arm.com; alexander.shish...@linux.intel.com;
> jo...@redhat.com; namhy...@kernel.org; irog...@google.com;
> kj...@linux.ibm.com
> Cc: linux-kernel@vger.kernel.org; linux...@openeuler.org; Joakim Zhang
> ; John Garry
> Subject: [PATCH] perf metricgroup: Fix system PMU metrics
>
> Joakim reports that getting "perf stat" for multiple system PMU metrics
> segfaults:
> ./perf stat -a -I 1000 -M imx8mm_ddr_write.all,imx8mm_ddr_write.all
> Segmentation fault
>
> While the same works without issue for a single metric.
>
> The logic in metricgroup__add_metric_sys_event_iter() is broken, in that
> add_metric() @m argument should be NULL for each new metric. Fix by not
> passing a holder for that, and rather make local in
> metricgroup__add_metric_sys_event_iter().
>
> Fixes: be335ec28efa ("perf metricgroup: Support adding metrics for system
> PMUs")
> Reported-by: Joakim Zhang
> Signed-off-by: John Garry
root@imx8mmevk:~# ./perf stat -a -I 1000 -M
imx8mm_ddr_read.all,imx8mm_ddr_write
.all
# time counts unit events
1.001446500 40832 imx8mm_ddr.read_cycles#638.0
KB imx8mm_ddr_read.all
1.001446500 16973 imx8mm_ddr.write_cycles #265.2
KB imx8mm_ddr_write.all
2.003150250 28836 imx8mm_ddr.read_cycles#450.6
KB imx8mm_ddr_read.all
2.003150250 6705 imx8mm_ddr.write_cycles #104.8
KB imx8mm_ddr_write.all
For this issue, Tested-by: Joakim Zhang
Hi John,
It seems have other issue compared to 5.10 kernel after switching to this
framework, below metric can't work.
"MetricExpr": "(( imx8_ddr0@read\\-cycles@ + imx8_ddr0@write\\-cycles@ ) * 4 *
4 / duration_time) / (750 * 100 * 4 * 4)"
After change to:
"MetricExpr": "(( imx8mm_ddr.read_cycles + imx8mm_ddr.write_cycles ) * 4 * 4 /
duration_time) / (750 * 100 * 4 * 4)",
Best Regards,
Joakim Zhang
> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> index
> ee94d3e8dd65..2e60ee170abc 100644
> --- a/tools/perf/util/metricgroup.c
> +++ b/tools/perf/util/metricgroup.c
> @@ -766,7 +766,6 @@ int __weak arch_get_runtimeparam(struct pmu_event
> *pe __maybe_unused) struct metricgroup_add_iter_data {
> struct list_head *metric_list;
> const char *metric;
> - struct metric **m;
> struct expr_ids *ids;
> int *ret;
> bool *has_match;
> @@ -1058,12 +1057,13 @@ static int
> metricgroup__add_metric_sys_event_iter(struct pmu_event *pe,
> void *data)
> {
> struct metricgroup_add_iter_data *d = data;
> + struct metric *m = NULL;
> int ret;
>
> if (!match_pe_metric(pe, d->metric))
> return 0;
>
> - ret = add_metric(d->metric_list, pe, d->metric_no_group, d->m, NULL,
> d->ids);
> + ret = add_metric(d->metric_list, pe, d->metric_no_group, &m, NULL,
> +d->ids);
> if (ret)
> return ret;
>
> @@ -1114,7 +1114,6 @@ static int metricgroup__add_metric(const char
> *metric, bool metric_no_group,
> .metric_list = &list,
> .metric = metric,
> .metric_no_group = metric_no_group,
> - .m = &m,
> .ids = &ids,
> .has_match = &has_match,
> .ret = &ret,
> --
> 2.26.2
RE: [PATCH V5 0/5] can: flexcan: add stop mode support for i.MX8QM
> -Original Message- > From: Marc Kleine-Budde > Sent: 2020年11月6日 19:33 > To: Joakim Zhang ; robh...@kernel.org; > shawn...@kernel.org; s.ha...@pengutronix.de > Cc: ker...@pengutronix.de; dl-linux-imx ; > linux-...@vger.kernel.org; net...@vger.kernel.org; > linux-kernel@vger.kernel.org > Subject: Re: [PATCH V5 0/5] can: flexcan: add stop mode support for i.MX8QM > > On 11/6/20 11:56 AM, Joakim Zhang wrote: > > Add stop mode support for i.MX8QM. > > > > ChangeLogs: > > V4->V5: > > * remove patch:firmware: imx: always export SCU symbols, since > > it done by commit: 95de5094f5ac firmware: imx: add dummy functions > > * rebase to fsl,flexcan.yaml > > > > V3->V4: > > * can_idx->scu_idx. > > * return imx_scu_get_handle(&priv->sc_ipc_handle); > > * failed_canregister->failed_setup_stop_mode. > > > > V2->V3: > > * define IMX_SC_R_CAN(x) in rsrc.h > > * remove error message on -EPROBE_DEFER. > > * split disable wakeup patch into separate one. > > > > V1->V2: > > * split ECC fix patches into separate patches. > > * free can dev if failed to setup stop mode. > > * disable wakeup on flexcan_remove. > > * add FLEXCAN_IMX_SC_R_CAN macro helper. > > * fsl,can-index->fsl,scu-index. > > * move fsl,scu-index and priv->can_idx into > > * flexcan_setup_stop_mode_scfw() > > * prove failed if failed to setup stop mode. > > > > Joakim Zhang (5): > > dt-bindings: can: flexcan: fix fsl,clk-source property > > added to linux-can/testing > > > dt-bindings: can: flexcan: add fsl,scu-index property to indicate a > > resource > > can: flexcan: rename macro FLEXCAN_QUIRK_SETUP_STOP_MODE -> > > FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR > > dt-bindings: firmware: add IMX_SC_R_CAN(x) macro for CAN > > can: flexcan: add CAN wakeup function for i.MX8QM > > The others go via linux-can-next/testing, once net/master is merged back to > net-next/master to have the yaml bindings. Hi Marc, How about below patches? I even can't see it in your linux-can-next/testing branch. Are these missed? dt-bindings: can: flexcan: add fsl,scu-index property to indicate a resource can: flexcan: add CAN wakeup function for i.MX8QM Best Regards, Joakim Zhang
RE: [PATCH v2 0/4] perf drivers: Add sysfs identifier file
> -Original Message- > From: John Garry > Sent: 2020年11月26日 0:00 > To: Will Deacon ; Joakim Zhang > ; zhangshao...@hisilicon.com; > mark.rutl...@arm.com; Frank Li ; robh...@kernel.org > Cc: catalin.mari...@arm.com; kernel-t...@android.com; a...@kernel.org; > linux-kernel@vger.kernel.org; linux-arm-ker...@lists.infradead.org; > irog...@google.com; jo...@redhat.com; devicet...@vger.kernel.org; > linux...@huawei.com > Subject: Re: [PATCH v2 0/4] perf drivers: Add sysfs identifier file > > On 25/11/2020 15:44, Will Deacon wrote: > > Applied the hisi and smmu parts to will (for-next/perf), thanks! > > > > [1/4] drivers/perf: hisi: Add identifier sysfs file > > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit. > > > kernel.org%2Fwill%2Fc%2Fac4511c9364c&data=04%7C01%7Cqiangqing.z > han > > > g%40nxp.com%7Cbc2c0da5b8c04caccd1708d8915b44fd%7C686ea1d3bc2b4c6 > fa92cd > > > 99c5c301635%7C0%7C0%7C637419168472681071%7CUnknown%7CTWFpbGZ > sb3d8eyJWI > > > joiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1 > 000&a > > > mp;sdata=W2ZFzaMWJgpQncG4HF2x8EaWc0FkYJ69E8rVaFikJEQ%3D&re > served=0 > > [...] > > [4/4] perf/smmuv3: Support sysfs identifier file > > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit. > > > kernel.org%2Fwill%2Fc%2F2c255223362e&data=04%7C01%7Cqiangqing. > zhan > > > g%40nxp.com%7Cbc2c0da5b8c04caccd1708d8915b44fd%7C686ea1d3bc2b4c6 > fa92cd > > > 99c5c301635%7C0%7C0%7C637419168472681071%7CUnknown%7CTWFpbGZ > sb3d8eyJWI > > > joiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1 > 000&a > > > mp;sdata=lfmEr2GO9iySRp24zWTObHkWAXEo3an6TxA%2BzXpDlvk%3D& > reserved > > =0 > > > > Thanks. > > > I've completely lost track of the imx ddr PMU, so I dropped those > > parts (patch 3/4 seemed to remove a compatible string from the driver?). > > > > 2/4 needed to be dropped anyway. > > @Joakim, can you resend 3/4? And I think that we should keep the explicit > support for "fsl,imx8m-ddr-pmu" as a good practice in imx_ddr_pmu_dt_ids[], > while also adding the soc-specific sub compat string support Yes, thanks John. I will follow up. Best Regards, Joakim Zhang > John
[PATCH V5 5/5] can: flexcan: add CAN wakeup function for i.MX8QM
The System Controller Firmware (SCFW) is a low-level system function
which runs on a dedicated Cortex-M core to provide power, clock, and
resource management. It exists on some i.MX8 processors. e.g. i.MX8QM
(QM, QP), and i.MX8QX (QXP, DX). SCU driver manages the IPC interface
between host CPU and the SCU firmware running on M4.
For i.MX8QM, stop mode request is controlled by System Controller Unit(SCU)
firmware, this patch introduces FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW quirk
for this function.
Signed-off-by: Joakim Zhang
---
drivers/net/can/flexcan.c | 123 --
1 file changed, 106 insertions(+), 17 deletions(-)
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 8f578c867493..1f2adbc606f5 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -9,6 +9,7 @@
//
// Based on code originally by Andrey Volkov
+#include
#include
#include
#include
@@ -17,6 +18,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -242,6 +244,8 @@
#define FLEXCAN_QUIRK_SUPPORT_FD BIT(9)
/* support memory detection and correction */
#define FLEXCAN_QUIRK_SUPPORT_ECC BIT(10)
+/* Setup stop mode with SCU firmware to support wakeup */
+#define FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW BIT(11)
/* Structure of the message buffer */
struct flexcan_mb {
@@ -347,6 +351,7 @@ struct flexcan_priv {
u8 mb_count;
u8 mb_size;
u8 clk_src; /* clock source of CAN Protocol Engine */
+ u8 scu_idx;
u64 rx_mask;
u64 tx_mask;
@@ -358,6 +363,9 @@ struct flexcan_priv {
struct regulator *reg_xceiver;
struct flexcan_stop_mode stm;
+ /* IPC handle when setup stop mode by System Controller firmware(scfw)
*/
+ struct imx_sc_ipc *sc_ipc_handle;
+
/* Read and Write APIs */
u32 (*read)(void __iomem *addr);
void (*write)(u32 val, void __iomem *addr);
@@ -387,7 +395,7 @@ static const struct flexcan_devtype_data
fsl_imx6q_devtype_data = {
static const struct flexcan_devtype_data fsl_imx8qm_devtype_data = {
.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
FLEXCAN_QUIRK_USE_OFF_TIMESTAMP |
FLEXCAN_QUIRK_BROKEN_PERR_STATE |
- FLEXCAN_QUIRK_SUPPORT_FD,
+ FLEXCAN_QUIRK_SUPPORT_FD | FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW,
};
static struct flexcan_devtype_data fsl_imx8mp_devtype_data = {
@@ -546,18 +554,42 @@ static void flexcan_enable_wakeup_irq(struct flexcan_priv
*priv, bool enable)
priv->write(reg_mcr, ®s->mcr);
}
+static int flexcan_stop_mode_enable_scfw(struct flexcan_priv *priv, bool
enabled)
+{
+ u8 idx = priv->scu_idx;
+ u32 rsrc_id, val;
+
+ rsrc_id = IMX_SC_R_CAN(idx);
+
+ if (enabled)
+ val = 1;
+ else
+ val = 0;
+
+ /* stop mode request via scu firmware */
+ return imx_sc_misc_set_control(priv->sc_ipc_handle, rsrc_id,
+ IMX_SC_C_IPG_STOP, val);
+}
+
static inline int flexcan_enter_stop_mode(struct flexcan_priv *priv)
{
struct flexcan_regs __iomem *regs = priv->regs;
u32 reg_mcr;
+ int ret;
reg_mcr = priv->read(®s->mcr);
reg_mcr |= FLEXCAN_MCR_SLF_WAK;
priv->write(reg_mcr, ®s->mcr);
/* enable stop request */
- regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
- 1 << priv->stm.req_bit, 1 << priv->stm.req_bit);
+ if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW) {
+ ret = flexcan_stop_mode_enable_scfw(priv, true);
+ if (ret < 0)
+ return ret;
+ } else {
+ regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
+ 1 << priv->stm.req_bit, 1 <<
priv->stm.req_bit);
+ }
return flexcan_low_power_enter_ack(priv);
}
@@ -566,10 +598,17 @@ static inline int flexcan_exit_stop_mode(struct
flexcan_priv *priv)
{
struct flexcan_regs __iomem *regs = priv->regs;
u32 reg_mcr;
+ int ret;
/* remove stop request */
- regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
- 1 << priv->stm.req_bit, 0);
+ if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW) {
+ ret = flexcan_stop_mode_enable_scfw(priv, false);
+ if (ret < 0)
+ return ret;
+ } else {
+ regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
+ 1 << priv->stm.req_bit, 0);
+ }
reg_mcr = priv->read(®s->mcr);
reg_mcr &= ~FLEXCAN_MCR_SLF_WAK;
@@ -1838,7 +1877,7 @@ static void unregister_flexcandev(struct net_device *dev)
[PATCH V5 4/5] dt-bindings: firmware: add IMX_SC_R_CAN(x) macro for CAN
Add IMX_SC_R_CAN(x) macro for CAN. Suggested-by: Marc Kleine-Budde Acked-by: Shawn Guo Signed-off-by: Joakim Zhang --- include/dt-bindings/firmware/imx/rsrc.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/dt-bindings/firmware/imx/rsrc.h b/include/dt-bindings/firmware/imx/rsrc.h index 54278d5c1856..43885056557c 100644 --- a/include/dt-bindings/firmware/imx/rsrc.h +++ b/include/dt-bindings/firmware/imx/rsrc.h @@ -111,6 +111,7 @@ #define IMX_SC_R_CAN_0 105 #define IMX_SC_R_CAN_1 106 #define IMX_SC_R_CAN_2 107 +#define IMX_SC_R_CAN(x)(IMX_SC_R_CAN_0 + (x)) #define IMX_SC_R_DMA_1_CH0 108 #define IMX_SC_R_DMA_1_CH1 109 #define IMX_SC_R_DMA_1_CH2 110 -- 2.17.1
[PATCH V5 1/5] dt-bindings: can: flexcan: fix fsl,clk-source property
Correct fsl,clk-source example since flexcan driver uses "of_property_read_u8"
to get this property.
Fixes: 9d733992772d ("dt-bindings: can: flexcan: add PE clock source property
to device tree")
Signed-off-by: Joakim Zhang
---
Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml
b/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml
index 43df15ba8fa4..8f4db883e16b 100644
--- a/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml
+++ b/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml
@@ -95,7 +95,7 @@ properties:
by default.
0: clock source 0 (oscillator clock)
1: clock source 1 (peripheral clock)
-$ref: /schemas/types.yaml#/definitions/uint32
+$ref: /schemas/types.yaml#/definitions/uint8
default: 1
minimum: 0
maximum: 1
@@ -120,7 +120,7 @@ examples:
interrupts = <48 0x2>;
interrupt-parent = <&mpic>;
clock-frequency = <2>;
-fsl,clk-source = <0>;
+fsl,clk-source = /bits/ 8 <0>;
};
- |
#include
--
2.17.1
[PATCH V5 3/5] can: flexcan: rename macro FLEXCAN_QUIRK_SETUP_STOP_MODE -> FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR
This patch intends to rename FLEXCAN_QUIRK_SETUP_STOP_MODE quirk
to FLEXCAN_QUIRK_SETUP_STOP_MODE_GRP for non-scu SoCs, coming patch will
add quirk for scu SoCs.
For non-scu SoCs, setup stop mode with GPR register.
For scu SoCs, setup stop mode with SCU firmware.
Signed-off-by: Joakim Zhang
---
drivers/net/can/flexcan.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 881799bd9c5e..8f578c867493 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -236,8 +236,8 @@
#define FLEXCAN_QUIRK_BROKEN_PERR_STATE BIT(6)
/* default to BE register access */
#define FLEXCAN_QUIRK_DEFAULT_BIG_ENDIAN BIT(7)
-/* Setup stop mode to support wakeup */
-#define FLEXCAN_QUIRK_SETUP_STOP_MODE BIT(8)
+/* Setup stop mode with GPR to support wakeup */
+#define FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR BIT(8)
/* Support CAN-FD mode */
#define FLEXCAN_QUIRK_SUPPORT_FD BIT(9)
/* support memory detection and correction */
@@ -381,7 +381,7 @@ static const struct flexcan_devtype_data
fsl_imx28_devtype_data = {
static const struct flexcan_devtype_data fsl_imx6q_devtype_data = {
.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
FLEXCAN_QUIRK_USE_OFF_TIMESTAMP |
FLEXCAN_QUIRK_BROKEN_PERR_STATE |
- FLEXCAN_QUIRK_SETUP_STOP_MODE,
+ FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR,
};
static const struct flexcan_devtype_data fsl_imx8qm_devtype_data = {
@@ -393,7 +393,7 @@ static const struct flexcan_devtype_data
fsl_imx8qm_devtype_data = {
static struct flexcan_devtype_data fsl_imx8mp_devtype_data = {
.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
FLEXCAN_QUIRK_DISABLE_MECR | FLEXCAN_QUIRK_USE_OFF_TIMESTAMP |
- FLEXCAN_QUIRK_BROKEN_PERR_STATE | FLEXCAN_QUIRK_SETUP_STOP_MODE
|
+ FLEXCAN_QUIRK_BROKEN_PERR_STATE |
FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR |
FLEXCAN_QUIRK_SUPPORT_FD | FLEXCAN_QUIRK_SUPPORT_ECC,
};
@@ -2043,7 +2043,7 @@ static int flexcan_probe(struct platform_device *pdev)
of_can_transceiver(dev);
devm_can_led_init(dev);
- if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE) {
+ if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR) {
err = flexcan_setup_stop_mode(pdev);
if (err)
dev_dbg(&pdev->dev, "failed to setup stop-mode\n");
--
2.17.1
[PATCH V5 2/5] dt-bindings: can: flexcan: add fsl,scu-index property to indicate a resource
For SoCs with SCU support, need setup stop mode via SCU firmware, so this property can help indicate a resource in SCU firmware. Signed-off-by: Joakim Zhang --- .../devicetree/bindings/net/can/fsl,flexcan.yaml | 11 +++ 1 file changed, 11 insertions(+) diff --git a/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml b/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml index 8f4db883e16b..2631dad8f85f 100644 --- a/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml +++ b/Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml @@ -105,6 +105,16 @@ properties: description: Enable CAN remote wakeup. + fsl,scu-index: +description: | + The scu index of CAN instance. + For SoCs with SCU support, need setup stop mode via SCU firmware, so this + property can help indicate a resource. It supports up to 3 CAN instances + now. +$ref: /schemas/types.yaml#/definitions/uint8 +minimum: 0 +maximum: 2 + required: - compatible - reg @@ -132,4 +142,5 @@ examples: clocks = <&clks 1>, <&clks 2>; clock-names = "ipg", "per"; fsl,stop-mode = <&gpr 0x34 28>; +fsl,scu-index = /bits/ 8 <1>; }; -- 2.17.1
[PATCH V5 0/5] can: flexcan: add stop mode support for i.MX8QM
Add stop mode support for i.MX8QM. ChangeLogs: V4->V5: * remove patch:firmware: imx: always export SCU symbols, since it done by commit: 95de5094f5ac firmware: imx: add dummy functions * rebase to fsl,flexcan.yaml V3->V4: * can_idx->scu_idx. * return imx_scu_get_handle(&priv->sc_ipc_handle); * failed_canregister->failed_setup_stop_mode. V2->V3: * define IMX_SC_R_CAN(x) in rsrc.h * remove error message on -EPROBE_DEFER. * split disable wakeup patch into separate one. V1->V2: * split ECC fix patches into separate patches. * free can dev if failed to setup stop mode. * disable wakeup on flexcan_remove. * add FLEXCAN_IMX_SC_R_CAN macro helper. * fsl,can-index->fsl,scu-index. * move fsl,scu-index and priv->can_idx into * flexcan_setup_stop_mode_scfw() * prove failed if failed to setup stop mode. Joakim Zhang (5): dt-bindings: can: flexcan: fix fsl,clk-source property dt-bindings: can: flexcan: add fsl,scu-index property to indicate a resource can: flexcan: rename macro FLEXCAN_QUIRK_SETUP_STOP_MODE -> FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR dt-bindings: firmware: add IMX_SC_R_CAN(x) macro for CAN can: flexcan: add CAN wakeup function for i.MX8QM .../bindings/net/can/fsl,flexcan.yaml | 15 +- drivers/net/can/flexcan.c | 131 +++--- include/dt-bindings/firmware/imx/rsrc.h | 1 + 3 files changed, 124 insertions(+), 23 deletions(-) -- 2.17.1
RE: [PATCH V2] arm64: dts: imx8mp-evk: add CAN support
> -Original Message-
> From: Krzysztof Kozlowski
> Sent: 2020年11月3日 15:39
> To: Joakim Zhang
> Cc: shawn...@kernel.org; s.ha...@pengutronix.de; feste...@gmail.com;
> dl-linux-imx ; devicet...@vger.kernel.org;
> linux-arm-ker...@lists.infradead.org; linux-kernel@vger.kernel.org;
> m...@pengutronix.de
> Subject: Re: [PATCH V2] arm64: dts: imx8mp-evk: add CAN support
>
> On Tue, Nov 03, 2020 at 01:23:12AM +, Joakim Zhang wrote:
> >
> > > -Original Message-
> > > From: Krzysztof Kozlowski
> > > Sent: 2020年11月2日 16:29
> > > To: Joakim Zhang
> > > Cc: shawn...@kernel.org; s.ha...@pengutronix.de;
> feste...@gmail.com;
> > > dl-linux-imx ; devicet...@vger.kernel.org;
> > > linux-arm-ker...@lists.infradead.org; linux-kernel@vger.kernel.org;
> > > m...@pengutronix.de
> > > Subject: Re: [PATCH V2] arm64: dts: imx8mp-evk: add CAN support
> > >
> > > On Mon, Nov 02, 2020 at 10:16:34AM +0800, Joakim Zhang wrote:
> > > > Add CAN device node and pinctrl on i.MX8MP evk board.
> > > >
> > > > Signed-off-by: Joakim Zhang
> > > > ---
> > > > ChangeLogs:
> > > > V1->V2:
> > > > * add missing space before '=',
> > > > fsl,clk-source= /bits/ 8 <0> -> fsl,clk-source = /bits/ 8 <0>
> > > > ---
> > > > arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 62
> > >
> > > > arch/arm64/boot/dts/freescale/imx8mp.dtsi| 30 ++
> > > > 2 files changed, 92 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> > > > b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> > > > index 908b92bb4dcd..b10dce8767a4 100644
> > > > --- a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> > > > +++ b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> > > > @@ -33,6 +33,28 @@
> > > > <0x1 0x 0 0xc000>;
> > > > };
> > > >
> > > > + reg_can1_stby: regulator-can1-stby {
> > > > + compatible = "regulator-fixed";
> > > > + regulator-name = "can1-stby";
> > > > + pinctrl-names = "default";
> > > > + pinctrl-0 = <&pinctrl_flexcan1_reg>;
> > > > + regulator-min-microvolt = <330>;
> > > > + regulator-max-microvolt = <330>;
> > > > + gpio = <&gpio5 5 GPIO_ACTIVE_HIGH>;
> > > > + enable-active-high;
> > > > + };
> > > > +
> > > > + reg_can2_stby: regulator-can2-stby {
> > > > + compatible = "regulator-fixed";
> > > > + regulator-name = "can2-stby";
> > > > + pinctrl-names = "default";
> > > > + pinctrl-0 = <&pinctrl_flexcan2_reg>;
> > > > + regulator-min-microvolt = <330>;
> > > > + regulator-max-microvolt = <330>;
> > > > + gpio = <&gpio4 27 GPIO_ACTIVE_HIGH>;
> > > > + enable-active-high;
> > > > + };
> > > > +
> > > > reg_usdhc2_vmmc: regulator-usdhc2 {
> > > > compatible = "regulator-fixed";
> > > > pinctrl-names = "default";
> > > > @@ -45,6 +67,20 @@
> > > > };
> > > > };
> > > >
> > > > +&flexcan1 {
> > > > + pinctrl-names = "default";
> > > > + pinctrl-0 = <&pinctrl_flexcan1>;
> > > > + xceiver-supply = <®_can1_stby>;
> > > > + status = "okay";
> > > > +};
> > > > +
> > > > +&flexcan2 {
> > > > + pinctrl-names = "default";
> > > > + pinctrl-0 = <&pinctrl_flexcan2>;
> > > > + xceiver-supply = <®_can2_stby>;
> > > > + status = "disabled";/* can2 pin conflict with pdm */ };
> > > > +
> > > > &fec {
> > > > pinctrl-names = "default";
> > > > pinctrl-0 = <&pinctrl_fec>;
> > &g
RE: [PATCH V2] arm64: dts: imx8mp-evk: add CAN support
> -Original Message-
> From: Krzysztof Kozlowski
> Sent: 2020年11月2日 16:29
> To: Joakim Zhang
> Cc: shawn...@kernel.org; s.ha...@pengutronix.de; feste...@gmail.com;
> dl-linux-imx ; devicet...@vger.kernel.org;
> linux-arm-ker...@lists.infradead.org; linux-kernel@vger.kernel.org;
> m...@pengutronix.de
> Subject: Re: [PATCH V2] arm64: dts: imx8mp-evk: add CAN support
>
> On Mon, Nov 02, 2020 at 10:16:34AM +0800, Joakim Zhang wrote:
> > Add CAN device node and pinctrl on i.MX8MP evk board.
> >
> > Signed-off-by: Joakim Zhang
> > ---
> > ChangeLogs:
> > V1->V2:
> > * add missing space before '=',
> > fsl,clk-source= /bits/ 8 <0> -> fsl,clk-source = /bits/ 8 <0>
> > ---
> > arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 62
>
> > arch/arm64/boot/dts/freescale/imx8mp.dtsi| 30 ++
> > 2 files changed, 92 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> > b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> > index 908b92bb4dcd..b10dce8767a4 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> > @@ -33,6 +33,28 @@
> > <0x1 0x 0 0xc000>;
> > };
> >
> > + reg_can1_stby: regulator-can1-stby {
> > + compatible = "regulator-fixed";
> > + regulator-name = "can1-stby";
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_flexcan1_reg>;
> > + regulator-min-microvolt = <330>;
> > + regulator-max-microvolt = <330>;
> > + gpio = <&gpio5 5 GPIO_ACTIVE_HIGH>;
> > + enable-active-high;
> > + };
> > +
> > + reg_can2_stby: regulator-can2-stby {
> > + compatible = "regulator-fixed";
> > + regulator-name = "can2-stby";
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_flexcan2_reg>;
> > + regulator-min-microvolt = <330>;
> > + regulator-max-microvolt = <330>;
> > + gpio = <&gpio4 27 GPIO_ACTIVE_HIGH>;
> > + enable-active-high;
> > + };
> > +
> > reg_usdhc2_vmmc: regulator-usdhc2 {
> > compatible = "regulator-fixed";
> > pinctrl-names = "default";
> > @@ -45,6 +67,20 @@
> > };
> > };
> >
> > +&flexcan1 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_flexcan1>;
> > + xceiver-supply = <®_can1_stby>;
> > + status = "okay";
> > +};
> > +
> > +&flexcan2 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_flexcan2>;
> > + xceiver-supply = <®_can2_stby>;
> > + status = "disabled";/* can2 pin conflict with pdm */ };
> > +
> > &fec {
> > pinctrl-names = "default";
> > pinctrl-0 = <&pinctrl_fec>;
> > @@ -144,6 +180,32 @@
> > >;
> > };
> >
> > + pinctrl_flexcan1: flexcan1grp {
> > + fsl,pins = <
> > + MX8MP_IOMUXC_SPDIF_RX__CAN1_RX 0x154
> > + MX8MP_IOMUXC_SPDIF_TX__CAN1_TX 0x154
> > + >;
> > + };
> > +
> > + pinctrl_flexcan2: flexcan2grp {
> > + fsl,pins = <
> > + MX8MP_IOMUXC_SAI5_MCLK__CAN2_RX 0x154
> > + MX8MP_IOMUXC_SAI5_RXD3__CAN2_TX 0x154
> > + >;
> > + };
> > +
> > + pinctrl_flexcan1_reg: flexcan1reggrp {
> > + fsl,pins = <
> > + MX8MP_IOMUXC_SPDIF_EXT_CLK__GPIO5_IO05 0x154 /*
> CAN1_STBY */
> > + >;
> > + };
> > +
> > + pinctrl_flexcan2_reg: flexcan2reggrp {
> > + fsl,pins = <
> > + MX8MP_IOMUXC_SAI2_MCLK__GPIO4_IO27 0x154
> /* CAN2_STBY */
> > + >;
> > + };
> > +
> > pinctrl_gpio_led: gpioledgrp {
> > fsl,pins = <
> > MX8MP_IOMUXC_NAND_READY_B__GPIO3_IO16 0x19
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > index 479312293036..ec
RE: [PATCH V4 1/6] firmware: imx: always export SCU symbols
> -Original Message-
> From: Shawn Guo
> Sent: 2020年11月1日 15:43
> To: Joakim Zhang
> Cc: m...@pengutronix.de; robh...@kernel.org; s.ha...@pengutronix.de;
> ker...@pengutronix.de; dl-linux-imx ; Ying Liu
> ; linux-...@vger.kernel.org; net...@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH V4 1/6] firmware: imx: always export SCU symbols
>
> On Wed, Oct 21, 2020 at 01:24:32PM +0800, Joakim Zhang wrote:
> > From: Liu Ying
> >
> > Always export SCU symbols for both SCU SoCs and non-SCU SoCs to avoid
> > build error.
> >
> > Signed-off-by: Liu Ying
> > Signed-off-by: Peng Fan
> > Signed-off-by: Joakim Zhang
> > ---
> > include/linux/firmware/imx/ipc.h | 15 +++
>
> Could you rebase it to my imx/drivers branch? There is one patch from Peng
> Fan that already changed ipc.h.
Hi Shawn,
Please ignore 1/6 patch since it has been done with Peng's patch. I will resend
flexcan patch set after Peng's patch goes into mainline.
Could you please review 5/6 patch which is suggested by Flexcan driver
maintainer? Thanks.
Best Regards,
Joakim Zhang
> Shawn
>
> > include/linux/firmware/imx/svc/misc.h | 23 +++
> > 2 files changed, 38 insertions(+)
> >
> > diff --git a/include/linux/firmware/imx/ipc.h
> > b/include/linux/firmware/imx/ipc.h
> > index 891057434858..300fa253fc30 100644
> > --- a/include/linux/firmware/imx/ipc.h
> > +++ b/include/linux/firmware/imx/ipc.h
> > @@ -34,6 +34,7 @@ struct imx_sc_rpc_msg {
> > uint8_t func;
> > };
> >
> > +#if IS_ENABLED(CONFIG_IMX_SCU)
> > /*
> > * This is an function to send an RPC message over an IPC channel.
> > * It is called by client-side SCFW API function shims.
> > @@ -55,4 +56,18 @@ int imx_scu_call_rpc(struct imx_sc_ipc *ipc, void
> *msg, bool have_resp);
> > * @return Returns an error code (0 = success, failed if < 0)
> > */
> > int imx_scu_get_handle(struct imx_sc_ipc **ipc);
> > +
> > +#else
> > +static inline int
> > +imx_scu_call_rpc(struct imx_sc_ipc *ipc, void *msg, bool have_resp) {
> > + return -EIO;
> > +}
> > +
> > +static inline int imx_scu_get_handle(struct imx_sc_ipc **ipc) {
> > + return -EIO;
> > +}
> > +#endif
> > +
> > #endif /* _SC_IPC_H */
> > diff --git a/include/linux/firmware/imx/svc/misc.h
> > b/include/linux/firmware/imx/svc/misc.h
> > index 031dd4d3c766..d255048f17de 100644
> > --- a/include/linux/firmware/imx/svc/misc.h
> > +++ b/include/linux/firmware/imx/svc/misc.h
> > @@ -46,6 +46,7 @@ enum imx_misc_func {
> > * Control Functions
> > */
> >
> > +#if IS_ENABLED(CONFIG_IMX_SCU)
> > int imx_sc_misc_set_control(struct imx_sc_ipc *ipc, u32 resource,
> > u8 ctrl, u32 val);
> >
> > @@ -55,4 +56,26 @@ int imx_sc_misc_get_control(struct imx_sc_ipc *ipc,
> > u32 resource, int imx_sc_pm_cpu_start(struct imx_sc_ipc *ipc, u32
> resource,
> > bool enable, u64 phys_addr);
> >
> > +#else
> > +static inline int
> > +imx_sc_misc_set_control(struct imx_sc_ipc *ipc, u32 resource,
> > + u8 ctrl, u32 val)
> > +{
> > + return -EIO;
> > +}
> > +
> > +static inline int
> > +imx_sc_misc_get_control(struct imx_sc_ipc *ipc, u32 resource,
> > + u8 ctrl, u32 *val)
> > +{
> > + return -EIO;
> > +}
> > +
> > +static inline int imx_sc_pm_cpu_start(struct imx_sc_ipc *ipc, u32 resource,
> > + bool enable, u64 phys_addr) {
> > + return -EIO;
> > +}
> > +#endif
> > +
> > #endif /* _SC_MISC_API_H */
> > --
> > 2.17.1
> >
[PATCH 2/3] arm64: dts: imx8mm-evk: add IR support
Add IR support on i.MX8MM EVK board.
Signed-off-by: Joakim Zhang
---
arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi | 14 ++
1 file changed, 14 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi
b/arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi
index fbfb57b195ad..6518f088b2c2 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi
@@ -41,6 +41,14 @@
enable-active-high;
};
+ ir-receiver {
+ compatible = "gpio-ir-receiver";
+ gpios = <&gpio1 13 GPIO_ACTIVE_LOW>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_ir>;
+ linux,autosuspend-period = <125>;
+ };
+
wm8524: audio-codec {
#sound-dai-cells = <0>;
compatible = "wlf,wm8524";
@@ -364,6 +372,12 @@
>;
};
+ pinctrl_ir: irgrp {
+ fsl,pins = <
+ MX8MM_IOMUXC_GPIO1_IO13_GPIO1_IO13 0x4f
+ >;
+ };
+
pinctrl_gpio_wlf: gpiowlfgrp {
fsl,pins = <
MX8MM_IOMUXC_I2C4_SDA_GPIO5_IO210xd6
--
2.17.1
[PATCH 3/3] arm64: dts: imx8mn-evk: add IR support
Add IR support on i.MX8MN EVK board.
Signed-off-by: Joakim Zhang
---
arch/arm64/boot/dts/freescale/imx8mn-evk.dtsi | 14 ++
1 file changed, 14 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8mn-evk.dtsi
b/arch/arm64/boot/dts/freescale/imx8mn-evk.dtsi
index fda890589a09..76d042a4cf09 100644
--- a/arch/arm64/boot/dts/freescale/imx8mn-evk.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mn-evk.dtsi
@@ -38,6 +38,14 @@
gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>;
enable-active-high;
};
+
+ ir-receiver {
+ compatible = "gpio-ir-receiver";
+ gpios = <&gpio1 13 GPIO_ACTIVE_LOW>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_ir>;
+ linux,autosuspend-period = <125>;
+ };
};
&fec1 {
@@ -202,6 +210,12 @@
>;
};
+ pinctrl_ir: irgrp {
+ fsl,pins = <
+ MX8MN_IOMUXC_GPIO1_IO13_GPIO1_IO13 0x4f
+ >;
+ };
+
pinctrl_i2c1: i2c1grp {
fsl,pins = <
MX8MN_IOMUXC_I2C1_SCL_I2C1_SCL 0x41c3
--
2.17.1
[PATCH 0/3] arm64: dts: imx8m: add IR support
Add IR support on i.MX8M platforms. Joakim Zhang (3): arm64: dts: imx8mq-evk: add linux,autosuspend-period property for IR arm64: dts: imx8mm-evk: add IR support arm64: dts: imx8mn-evk: add IR support arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi | 14 ++ arch/arm64/boot/dts/freescale/imx8mn-evk.dtsi | 14 ++ arch/arm64/boot/dts/freescale/imx8mq-evk.dts | 1 + 3 files changed, 29 insertions(+) -- 2.17.1
[PATCH 1/3] arm64: dts: imx8mq-evk: add linux,autosuspend-period property for IR
Add linux,autosuspend-period property for IR, details please refer to:
commit ff1c9223b7b8 ("media: rc: gpio-ir-recv: add QoS support for cpuidle
system")
Signed-off-by: Joakim Zhang
---
arch/arm64/boot/dts/freescale/imx8mq-evk.dts | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
index 2418cca00bc5..e89d1ba8b77e 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
@@ -57,6 +57,7 @@
gpios = <&gpio1 12 GPIO_ACTIVE_LOW>;
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_ir>;
+ linux,autosuspend-period = <125>;
};
wm8524: audio-codec {
--
2.17.1
[PATCH V2] arm64: dts: imx8mp-evk: add CAN support
Add CAN device node and pinctrl on i.MX8MP evk board.
Signed-off-by: Joakim Zhang
---
ChangeLogs:
V1->V2:
* add missing space before '=',
fsl,clk-source= /bits/ 8 <0> -> fsl,clk-source = /bits/ 8 <0>
---
arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 62
arch/arm64/boot/dts/freescale/imx8mp.dtsi| 30 ++
2 files changed, 92 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
index 908b92bb4dcd..b10dce8767a4 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
@@ -33,6 +33,28 @@
<0x1 0x 0 0xc000>;
};
+ reg_can1_stby: regulator-can1-stby {
+ compatible = "regulator-fixed";
+ regulator-name = "can1-stby";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_flexcan1_reg>;
+ regulator-min-microvolt = <330>;
+ regulator-max-microvolt = <330>;
+ gpio = <&gpio5 5 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ };
+
+ reg_can2_stby: regulator-can2-stby {
+ compatible = "regulator-fixed";
+ regulator-name = "can2-stby";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_flexcan2_reg>;
+ regulator-min-microvolt = <330>;
+ regulator-max-microvolt = <330>;
+ gpio = <&gpio4 27 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ };
+
reg_usdhc2_vmmc: regulator-usdhc2 {
compatible = "regulator-fixed";
pinctrl-names = "default";
@@ -45,6 +67,20 @@
};
};
+&flexcan1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_flexcan1>;
+ xceiver-supply = <®_can1_stby>;
+ status = "okay";
+};
+
+&flexcan2 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_flexcan2>;
+ xceiver-supply = <®_can2_stby>;
+ status = "disabled";/* can2 pin conflict with pdm */
+};
+
&fec {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_fec>;
@@ -144,6 +180,32 @@
>;
};
+ pinctrl_flexcan1: flexcan1grp {
+ fsl,pins = <
+ MX8MP_IOMUXC_SPDIF_RX__CAN1_RX 0x154
+ MX8MP_IOMUXC_SPDIF_TX__CAN1_TX 0x154
+ >;
+ };
+
+ pinctrl_flexcan2: flexcan2grp {
+ fsl,pins = <
+ MX8MP_IOMUXC_SAI5_MCLK__CAN2_RX 0x154
+ MX8MP_IOMUXC_SAI5_RXD3__CAN2_TX 0x154
+ >;
+ };
+
+ pinctrl_flexcan1_reg: flexcan1reggrp {
+ fsl,pins = <
+ MX8MP_IOMUXC_SPDIF_EXT_CLK__GPIO5_IO05 0x154 /*
CAN1_STBY */
+ >;
+ };
+
+ pinctrl_flexcan2_reg: flexcan2reggrp {
+ fsl,pins = <
+ MX8MP_IOMUXC_SAI2_MCLK__GPIO4_IO27 0x154 /*
CAN2_STBY */
+ >;
+ };
+
pinctrl_gpio_led: gpioledgrp {
fsl,pins = <
MX8MP_IOMUXC_NAND_READY_B__GPIO3_IO16 0x19
diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index 479312293036..ecccfbb4f5ad 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -552,6 +552,36 @@
status = "disabled";
};
+ flexcan1: can@308c {
+ compatible = "fsl,imx8mp-flexcan",
"fsl,imx6q-flexcan";
+ reg = <0x308c 0x1>;
+ interrupts = ;
+ clocks = <&clk IMX8MP_CLK_IPG_ROOT>,
+<&clk IMX8MP_CLK_CAN1_ROOT>;
+ clock-names = "ipg", "per";
+ assigned-clocks = <&clk IMX8MP_CLK_CAN1>;
+ assigned-clock-parents = <&clk
IMX8MP_SYS_PLL1_40M>;
+ assigned-clock-rates = <4000>;
+ fsl,clk-source = /bits/ 8 <0>;
+ fsl,stop-mode = <&gpr 0x10 4>;
+ status = "disabled";
+ };
+
+ flexcan2: can@308d {
+
RE: [PATCH V4 0/6] can: flexcan: add stop mode support for i.MX8QM
Gentle Ping... @shawn...@kernel.org, Could you please help review patch 1/6 and 5/6 in this patch set? Best Regards, Joakim Zhang > -Original Message- > From: Joakim Zhang > Sent: 2020年10月21日 13:33 > To: m...@pengutronix.de; robh...@kernel.org; shawn...@kernel.org; > s.ha...@pengutronix.de > Cc: ker...@pengutronix.de; dl-linux-imx ; Ying Liu > ; linux-...@vger.kernel.org; net...@vger.kernel.org; > linux-kernel@vger.kernel.org > Subject: RE: [PATCH V4 0/6] can: flexcan: add stop mode support for i.MX8QM > > > > -Original Message- > > From: Joakim Zhang > > Sent: 2020年10月21日 13:25 > > To: m...@pengutronix.de; robh...@kernel.org; shawn...@kernel.org; > > s.ha...@pengutronix.de > > Cc: ker...@pengutronix.de; dl-linux-imx ; Ying Liu > > ; linux-...@vger.kernel.org; > > net...@vger.kernel.org; linux-kernel@vger.kernel.org > > Subject: [PATCH V4 0/6] can: flexcan: add stop mode support for > > i.MX8QM > > > > The first patch from Liu Ying aims to export SCU symbols for SoCs w/wo > > SCU, so that no need to check CONFIG_IMX_SCU in the specific driver. > > > > The following patches are flexcan fixes and add stop mode support for > > i.MX8QM. > > Hi Shawnguo, > > Could you please help review patch 1/6 and 5/6? Since flexcan driver depends > on these. Thanks. > > For patch 1/6, it will benefit other drivers which cover SoCs w/wo SCU, such > as > i.MX Ethernet Controller driver (drivers/net/ethernet/freescale/fec_main.c). > > Best Regards, > Joakim Zhang
[PATCH] arm64: dts: imx8mp-evk: add CAN support
Add CAN device node and pinctrl on i.MX8MP evk board.
Signed-off-by: Joakim Zhang
---
arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 62
arch/arm64/boot/dts/freescale/imx8mp.dtsi| 30 ++
2 files changed, 92 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
index ad66f1286d95..85aaed7dc4bc 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
@@ -33,6 +33,28 @@
<0x1 0x 0 0xc000>;
};
+ reg_can1_stby: regulator-can1-stby {
+ compatible = "regulator-fixed";
+ regulator-name = "can1-stby";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_flexcan1_reg>;
+ regulator-min-microvolt = <330>;
+ regulator-max-microvolt = <330>;
+ gpio = <&gpio5 5 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ };
+
+ reg_can2_stby: regulator-can2-stby {
+ compatible = "regulator-fixed";
+ regulator-name = "can2-stby";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_flexcan2_reg>;
+ regulator-min-microvolt = <330>;
+ regulator-max-microvolt = <330>;
+ gpio = <&gpio4 27 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ };
+
reg_usdhc2_vmmc: regulator-usdhc2 {
compatible = "regulator-fixed";
pinctrl-names = "default";
@@ -45,6 +67,20 @@
};
};
+&flexcan1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_flexcan1>;
+ xceiver-supply = <®_can1_stby>;
+ status = "okay";
+};
+
+&flexcan2 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_flexcan2>;
+ xceiver-supply = <®_can2_stby>;
+ status = "disabled";/* can2 pin conflict with pdm */
+};
+
&fec {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_fec>;
@@ -144,6 +180,32 @@
>;
};
+ pinctrl_flexcan1: flexcan1grp {
+ fsl,pins = <
+ MX8MP_IOMUXC_SPDIF_RX__CAN1_RX 0x154
+ MX8MP_IOMUXC_SPDIF_TX__CAN1_TX 0x154
+ >;
+ };
+
+ pinctrl_flexcan2: flexcan2grp {
+ fsl,pins = <
+ MX8MP_IOMUXC_SAI5_MCLK__CAN2_RX 0x154
+ MX8MP_IOMUXC_SAI5_RXD3__CAN2_TX 0x154
+ >;
+ };
+
+ pinctrl_flexcan1_reg: flexcan1reggrp {
+ fsl,pins = <
+ MX8MP_IOMUXC_SPDIF_EXT_CLK__GPIO5_IO05 0x154 /*
CAN1_STBY */
+ >;
+ };
+
+ pinctrl_flexcan2_reg: flexcan2reggrp {
+ fsl,pins = <
+ MX8MP_IOMUXC_SAI2_MCLK__GPIO4_IO27 0x154 /*
CAN2_STBY */
+ >;
+ };
+
pinctrl_gpio_led: gpioledgrp {
fsl,pins = <
MX8MP_IOMUXC_NAND_READY_B__GPIO3_IO16 0x19
diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index 6038f66aefc1..cc123a5e3f7e 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -545,6 +545,36 @@
status = "disabled";
};
+ flexcan1: can@308c {
+ compatible = "fsl,imx8mp-flexcan",
"fsl,imx6q-flexcan";
+ reg = <0x308c 0x1>;
+ interrupts = ;
+ clocks = <&clk IMX8MP_CLK_IPG_ROOT>,
+<&clk IMX8MP_CLK_CAN1_ROOT>;
+ clock-names = "ipg", "per";
+ assigned-clocks = <&clk IMX8MP_CLK_CAN1>;
+ assigned-clock-parents = <&clk
IMX8MP_SYS_PLL1_40M>;
+ assigned-clock-rates = <4000>;
+ fsl,clk-source= /bits/ 8 <0>;
+ fsl,stop-mode = <&gpr 0x10 4>;
+ status = "disabled";
+ };
+
+ flexcan2: can@308d {
+ compatible = "fsl,imx8mp-flexcan",
"fsl,imx6q-flexcan";
+
RE: [PATCH V4 0/6] can: flexcan: add stop mode support for i.MX8QM
> -Original Message- > From: Joakim Zhang > Sent: 2020年10月21日 13:25 > To: m...@pengutronix.de; robh...@kernel.org; shawn...@kernel.org; > s.ha...@pengutronix.de > Cc: ker...@pengutronix.de; dl-linux-imx ; Ying Liu > ; linux-...@vger.kernel.org; net...@vger.kernel.org; > linux-kernel@vger.kernel.org > Subject: [PATCH V4 0/6] can: flexcan: add stop mode support for i.MX8QM > > The first patch from Liu Ying aims to export SCU symbols for SoCs w/wo SCU, so > that no need to check CONFIG_IMX_SCU in the specific driver. > > The following patches are flexcan fixes and add stop mode support for > i.MX8QM. Hi Shawnguo, Could you please help review patch 1/6 and 5/6? Since flexcan driver depends on these. Thanks. For patch 1/6, it will benefit other drivers which cover SoCs w/wo SCU, such as i.MX Ethernet Controller driver (drivers/net/ethernet/freescale/fec_main.c). Best Regards, Joakim Zhang
[PATCH V4 6/6] can: flexcan: add CAN wakeup function for i.MX8QM
The System Controller Firmware (SCFW) is a low-level system function
which runs on a dedicated Cortex-M core to provide power, clock, and
resource management. It exists on some i.MX8 processors. e.g. i.MX8QM
(QM, QP), and i.MX8QX (QXP, DX). SCU driver manages the IPC interface
between host CPU and the SCU firmware running on M4.
For i.MX8QM, stop mode request is controlled by System Controller Unit(SCU)
firmware, this patch introduces FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW quirk
for this function.
Signed-off-by: Joakim Zhang
---
drivers/net/can/flexcan.c | 123 --
1 file changed, 106 insertions(+), 17 deletions(-)
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 8f578c867493..1f2adbc606f5 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -9,6 +9,7 @@
//
// Based on code originally by Andrey Volkov
+#include
#include
#include
#include
@@ -17,6 +18,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -242,6 +244,8 @@
#define FLEXCAN_QUIRK_SUPPORT_FD BIT(9)
/* support memory detection and correction */
#define FLEXCAN_QUIRK_SUPPORT_ECC BIT(10)
+/* Setup stop mode with SCU firmware to support wakeup */
+#define FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW BIT(11)
/* Structure of the message buffer */
struct flexcan_mb {
@@ -347,6 +351,7 @@ struct flexcan_priv {
u8 mb_count;
u8 mb_size;
u8 clk_src; /* clock source of CAN Protocol Engine */
+ u8 scu_idx;
u64 rx_mask;
u64 tx_mask;
@@ -358,6 +363,9 @@ struct flexcan_priv {
struct regulator *reg_xceiver;
struct flexcan_stop_mode stm;
+ /* IPC handle when setup stop mode by System Controller firmware(scfw)
*/
+ struct imx_sc_ipc *sc_ipc_handle;
+
/* Read and Write APIs */
u32 (*read)(void __iomem *addr);
void (*write)(u32 val, void __iomem *addr);
@@ -387,7 +395,7 @@ static const struct flexcan_devtype_data
fsl_imx6q_devtype_data = {
static const struct flexcan_devtype_data fsl_imx8qm_devtype_data = {
.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
FLEXCAN_QUIRK_USE_OFF_TIMESTAMP |
FLEXCAN_QUIRK_BROKEN_PERR_STATE |
- FLEXCAN_QUIRK_SUPPORT_FD,
+ FLEXCAN_QUIRK_SUPPORT_FD | FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW,
};
static struct flexcan_devtype_data fsl_imx8mp_devtype_data = {
@@ -546,18 +554,42 @@ static void flexcan_enable_wakeup_irq(struct flexcan_priv
*priv, bool enable)
priv->write(reg_mcr, ®s->mcr);
}
+static int flexcan_stop_mode_enable_scfw(struct flexcan_priv *priv, bool
enabled)
+{
+ u8 idx = priv->scu_idx;
+ u32 rsrc_id, val;
+
+ rsrc_id = IMX_SC_R_CAN(idx);
+
+ if (enabled)
+ val = 1;
+ else
+ val = 0;
+
+ /* stop mode request via scu firmware */
+ return imx_sc_misc_set_control(priv->sc_ipc_handle, rsrc_id,
+ IMX_SC_C_IPG_STOP, val);
+}
+
static inline int flexcan_enter_stop_mode(struct flexcan_priv *priv)
{
struct flexcan_regs __iomem *regs = priv->regs;
u32 reg_mcr;
+ int ret;
reg_mcr = priv->read(®s->mcr);
reg_mcr |= FLEXCAN_MCR_SLF_WAK;
priv->write(reg_mcr, ®s->mcr);
/* enable stop request */
- regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
- 1 << priv->stm.req_bit, 1 << priv->stm.req_bit);
+ if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW) {
+ ret = flexcan_stop_mode_enable_scfw(priv, true);
+ if (ret < 0)
+ return ret;
+ } else {
+ regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
+ 1 << priv->stm.req_bit, 1 <<
priv->stm.req_bit);
+ }
return flexcan_low_power_enter_ack(priv);
}
@@ -566,10 +598,17 @@ static inline int flexcan_exit_stop_mode(struct
flexcan_priv *priv)
{
struct flexcan_regs __iomem *regs = priv->regs;
u32 reg_mcr;
+ int ret;
/* remove stop request */
- regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
- 1 << priv->stm.req_bit, 0);
+ if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW) {
+ ret = flexcan_stop_mode_enable_scfw(priv, false);
+ if (ret < 0)
+ return ret;
+ } else {
+ regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
+ 1 << priv->stm.req_bit, 0);
+ }
reg_mcr = priv->read(®s->mcr);
reg_mcr &= ~FLEXCAN_MCR_SLF_WAK;
@@ -1838,7 +1877,7 @@ static void unregister_flexcandev(struct net_device *dev)
[PATCH V4 0/6] can: flexcan: add stop mode support for i.MX8QM
The first patch from Liu Ying aims to export SCU symbols for SoCs w/wo SCU, so that no need to check CONFIG_IMX_SCU in the specific driver. The following patches are flexcan fixes and add stop mode support for i.MX8QM. ChangeLogs: V3->V4: * can_idx->scu_idx. * return imx_scu_get_handle(&priv->sc_ipc_handle); * failed_canregister->failed_setup_stop_mode. V2->V3: * define IMX_SC_R_CAN(x) in rsrc.h * remove error message on -EPROBE_DEFER. * split disable wakeup patch into separate one. V1->V2: * split ECC fix patches into separate patches. * free can dev if failed to setup stop mode. * disable wakeup on flexcan_remove. * add FLEXCAN_IMX_SC_R_CAN macro helper. * fsl,can-index->fsl,scu-index. * move fsl,scu-index and priv->can_idx into * flexcan_setup_stop_mode_scfw() * prove failed if failed to setup stop mode. Joakim Zhang (5): dt-bindings: can: flexcan: fix fsl,clk-source property dt-bindings: can: flexcan: add fsl,scu-index property to indicate a resource can: flexcan: rename macro FLEXCAN_QUIRK_SETUP_STOP_MODE -> FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR dt-bindings: firmware: add IMX_SC_R_CAN(x) macro for CAN can: flexcan: add CAN wakeup function for i.MX8QM Liu Ying (1): firmware: imx: always export SCU symbols .../bindings/net/can/fsl-flexcan.txt | 8 +- drivers/net/can/flexcan.c | 131 +++--- include/dt-bindings/firmware/imx/rsrc.h | 1 + include/linux/firmware/imx/ipc.h | 15 ++ include/linux/firmware/imx/svc/misc.h | 23 +++ 5 files changed, 156 insertions(+), 22 deletions(-) -- 2.17.1
[PATCH V4 5/6] dt-bindings: firmware: add IMX_SC_R_CAN(x) macro for CAN
Add IMX_SC_R_CAN(x) macro for CAN. Suggested-by: Marc Kleine-Budde Signed-off-by: Joakim Zhang --- include/dt-bindings/firmware/imx/rsrc.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/dt-bindings/firmware/imx/rsrc.h b/include/dt-bindings/firmware/imx/rsrc.h index 54278d5c1856..43885056557c 100644 --- a/include/dt-bindings/firmware/imx/rsrc.h +++ b/include/dt-bindings/firmware/imx/rsrc.h @@ -111,6 +111,7 @@ #define IMX_SC_R_CAN_0 105 #define IMX_SC_R_CAN_1 106 #define IMX_SC_R_CAN_2 107 +#define IMX_SC_R_CAN(x)(IMX_SC_R_CAN_0 + (x)) #define IMX_SC_R_DMA_1_CH0 108 #define IMX_SC_R_DMA_1_CH1 109 #define IMX_SC_R_DMA_1_CH2 110 -- 2.17.1
[PATCH V4 1/6] firmware: imx: always export SCU symbols
From: Liu Ying
Always export SCU symbols for both SCU SoCs and non-SCU SoCs to avoid
build error.
Signed-off-by: Liu Ying
Signed-off-by: Peng Fan
Signed-off-by: Joakim Zhang
---
include/linux/firmware/imx/ipc.h | 15 +++
include/linux/firmware/imx/svc/misc.h | 23 +++
2 files changed, 38 insertions(+)
diff --git a/include/linux/firmware/imx/ipc.h b/include/linux/firmware/imx/ipc.h
index 891057434858..300fa253fc30 100644
--- a/include/linux/firmware/imx/ipc.h
+++ b/include/linux/firmware/imx/ipc.h
@@ -34,6 +34,7 @@ struct imx_sc_rpc_msg {
uint8_t func;
};
+#if IS_ENABLED(CONFIG_IMX_SCU)
/*
* This is an function to send an RPC message over an IPC channel.
* It is called by client-side SCFW API function shims.
@@ -55,4 +56,18 @@ int imx_scu_call_rpc(struct imx_sc_ipc *ipc, void *msg, bool
have_resp);
* @return Returns an error code (0 = success, failed if < 0)
*/
int imx_scu_get_handle(struct imx_sc_ipc **ipc);
+
+#else
+static inline int
+imx_scu_call_rpc(struct imx_sc_ipc *ipc, void *msg, bool have_resp)
+{
+ return -EIO;
+}
+
+static inline int imx_scu_get_handle(struct imx_sc_ipc **ipc)
+{
+ return -EIO;
+}
+#endif
+
#endif /* _SC_IPC_H */
diff --git a/include/linux/firmware/imx/svc/misc.h
b/include/linux/firmware/imx/svc/misc.h
index 031dd4d3c766..d255048f17de 100644
--- a/include/linux/firmware/imx/svc/misc.h
+++ b/include/linux/firmware/imx/svc/misc.h
@@ -46,6 +46,7 @@ enum imx_misc_func {
* Control Functions
*/
+#if IS_ENABLED(CONFIG_IMX_SCU)
int imx_sc_misc_set_control(struct imx_sc_ipc *ipc, u32 resource,
u8 ctrl, u32 val);
@@ -55,4 +56,26 @@ int imx_sc_misc_get_control(struct imx_sc_ipc *ipc, u32
resource,
int imx_sc_pm_cpu_start(struct imx_sc_ipc *ipc, u32 resource,
bool enable, u64 phys_addr);
+#else
+static inline int
+imx_sc_misc_set_control(struct imx_sc_ipc *ipc, u32 resource,
+ u8 ctrl, u32 val)
+{
+ return -EIO;
+}
+
+static inline int
+imx_sc_misc_get_control(struct imx_sc_ipc *ipc, u32 resource,
+ u8 ctrl, u32 *val)
+{
+ return -EIO;
+}
+
+static inline int imx_sc_pm_cpu_start(struct imx_sc_ipc *ipc, u32 resource,
+ bool enable, u64 phys_addr)
+{
+ return -EIO;
+}
+#endif
+
#endif /* _SC_MISC_API_H */
--
2.17.1
[PATCH V4 2/6] dt-bindings: can: flexcan: fix fsl,clk-source property
Correct fsl,clk-source example since flexcan driver uses "of_property_read_u8"
to get this property.
Fixes: 9d733992772d ("dt-bindings: can: flexcan: add PE clock source property
to device tree")
Signed-off-by: Joakim Zhang
---
Documentation/devicetree/bindings/net/can/fsl-flexcan.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
index e10b6eb955e1..6af67f5e581c 100644
--- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
+++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
@@ -53,5 +53,5 @@ Example:
interrupts = <48 0x2>;
interrupt-parent = <&mpic>;
clock-frequency = <2>; // filled in by bootloader
- fsl,clk-source = <0>; // select clock source 0 for PE
+ fsl,clk-source = /bits/ 8 <0>; // select clock source 0 for PE
};
--
2.17.1
[PATCH V4 4/6] can: flexcan: rename macro FLEXCAN_QUIRK_SETUP_STOP_MODE -> FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR
This patch intends to rename FLEXCAN_QUIRK_SETUP_STOP_MODE quirk
to FLEXCAN_QUIRK_SETUP_STOP_MODE_GRP for non-scu SoCs, coming patch will
add quirk for scu SoCs.
For non-scu SoCs, setup stop mode with GPR register.
For scu SoCs, setup stop mode with SCU firmware.
Signed-off-by: Joakim Zhang
---
drivers/net/can/flexcan.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 881799bd9c5e..8f578c867493 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -236,8 +236,8 @@
#define FLEXCAN_QUIRK_BROKEN_PERR_STATE BIT(6)
/* default to BE register access */
#define FLEXCAN_QUIRK_DEFAULT_BIG_ENDIAN BIT(7)
-/* Setup stop mode to support wakeup */
-#define FLEXCAN_QUIRK_SETUP_STOP_MODE BIT(8)
+/* Setup stop mode with GPR to support wakeup */
+#define FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR BIT(8)
/* Support CAN-FD mode */
#define FLEXCAN_QUIRK_SUPPORT_FD BIT(9)
/* support memory detection and correction */
@@ -381,7 +381,7 @@ static const struct flexcan_devtype_data
fsl_imx28_devtype_data = {
static const struct flexcan_devtype_data fsl_imx6q_devtype_data = {
.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
FLEXCAN_QUIRK_USE_OFF_TIMESTAMP |
FLEXCAN_QUIRK_BROKEN_PERR_STATE |
- FLEXCAN_QUIRK_SETUP_STOP_MODE,
+ FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR,
};
static const struct flexcan_devtype_data fsl_imx8qm_devtype_data = {
@@ -393,7 +393,7 @@ static const struct flexcan_devtype_data
fsl_imx8qm_devtype_data = {
static struct flexcan_devtype_data fsl_imx8mp_devtype_data = {
.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
FLEXCAN_QUIRK_DISABLE_MECR | FLEXCAN_QUIRK_USE_OFF_TIMESTAMP |
- FLEXCAN_QUIRK_BROKEN_PERR_STATE | FLEXCAN_QUIRK_SETUP_STOP_MODE
|
+ FLEXCAN_QUIRK_BROKEN_PERR_STATE |
FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR |
FLEXCAN_QUIRK_SUPPORT_FD | FLEXCAN_QUIRK_SUPPORT_ECC,
};
@@ -2043,7 +2043,7 @@ static int flexcan_probe(struct platform_device *pdev)
of_can_transceiver(dev);
devm_can_led_init(dev);
- if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE) {
+ if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR) {
err = flexcan_setup_stop_mode(pdev);
if (err)
dev_dbg(&pdev->dev, "failed to setup stop-mode\n");
--
2.17.1
[PATCH V4 3/6] dt-bindings: can: flexcan: add fsl,scu-index property to indicate a resource
For SoCs with SCU support, need setup stop mode via SCU firmware, so this property can help indicate a resource in SCU firmware. Signed-off-by: Joakim Zhang --- Documentation/devicetree/bindings/net/can/fsl-flexcan.txt | 6 ++ 1 file changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt index 6af67f5e581c..38a7da4fef3f 100644 --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt +++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt @@ -43,6 +43,11 @@ Optional properties: 0: clock source 0 (oscillator clock) 1: clock source 1 (peripheral clock) +- fsl,scu-index: The scu index of CAN instance. + For SoCs with SCU support, need setup stop mode via SCU firmware, + so this property can help indicate a resource. It supports up to + 3 CAN instances now, so the value should be 0, 1, or 2. + - wakeup-source: enable CAN remote wakeup Example: @@ -54,4 +59,5 @@ Example: interrupt-parent = <&mpic>; clock-frequency = <2>; // filled in by bootloader fsl,clk-source = /bits/ 8 <0>; // select clock source 0 for PE + fsl,scu-index = /bits/ 8 <1>; // the second CAN instance }; -- 2.17.1
RE: [PATCH V3 06/10] can: flexcan: disable wakeup in flexcan_remove()
Hi Marc,
> -Original Message-
> From: Marc Kleine-Budde
> Sent: 2020年10月20日 17:31
> To: Joakim Zhang ; robh...@kernel.org;
> shawn...@kernel.org; s.ha...@pengutronix.de
> Cc: ker...@pengutronix.de; dl-linux-imx ; Ying Liu
> ; linux-...@vger.kernel.org; Pankaj Bansal
> ; net...@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH V3 06/10] can: flexcan: disable wakeup in flexcan_remove()
>
> On 10/20/20 5:53 PM, Joakim Zhang wrote:
> > Disable wakeup in flexcan_remove().
>
> The patch looks good, please explain why this is needed.
Okay, Can I resend this patch individually?
Joakim
> Marc
>
> >
> > Fixes: de3578c198c6 ("can: flexcan: add self wakeup support")
> > Fixes: 915f9666421c ("can: flexcan: add support for DT property
> > 'wakeup-source'")
> > Signed-off-by: Joakim Zhang
> > ---
> > drivers/net/can/flexcan.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> > index 06f94b6f0ebe..881799bd9c5e 100644
> > --- a/drivers/net/can/flexcan.c
> > +++ b/drivers/net/can/flexcan.c
> > @@ -2062,6 +2062,8 @@ static int flexcan_remove(struct platform_device
> > *pdev) {
> > struct net_device *dev = platform_get_drvdata(pdev);
> >
> > + device_set_wakeup_enable(&pdev->dev, false);
> > + device_set_wakeup_capable(&pdev->dev, false);
> > unregister_flexcandev(dev);
> > pm_runtime_disable(&pdev->dev);
> > free_candev(dev);
> >
>
>
> --
> Pengutronix e.K. | Marc Kleine-Budde |
> Embedded Linux | https://www.pengutronix.de |
> Vertretung West/Dortmund | Phone: +49-231-2826-924 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
[PATCH V3 08/10] can: flexcan: rename macro FLEXCAN_QUIRK_SETUP_STOP_MODE -> FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR
This patch intends to rename FLEXCAN_QUIRK_SETUP_STOP_MODE quirk
to FLEXCAN_QUIRK_SETUP_STOP_MODE_GRP for non-scu SoCs, coming patch will
add quirk for scu SoCs.
For non-scu SoCs, setup stop mode with GPR register.
For scu SoCs, setup stop mode with SCU firmware.
Signed-off-by: Joakim Zhang
---
drivers/net/can/flexcan.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 881799bd9c5e..8f578c867493 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -236,8 +236,8 @@
#define FLEXCAN_QUIRK_BROKEN_PERR_STATE BIT(6)
/* default to BE register access */
#define FLEXCAN_QUIRK_DEFAULT_BIG_ENDIAN BIT(7)
-/* Setup stop mode to support wakeup */
-#define FLEXCAN_QUIRK_SETUP_STOP_MODE BIT(8)
+/* Setup stop mode with GPR to support wakeup */
+#define FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR BIT(8)
/* Support CAN-FD mode */
#define FLEXCAN_QUIRK_SUPPORT_FD BIT(9)
/* support memory detection and correction */
@@ -381,7 +381,7 @@ static const struct flexcan_devtype_data
fsl_imx28_devtype_data = {
static const struct flexcan_devtype_data fsl_imx6q_devtype_data = {
.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
FLEXCAN_QUIRK_USE_OFF_TIMESTAMP |
FLEXCAN_QUIRK_BROKEN_PERR_STATE |
- FLEXCAN_QUIRK_SETUP_STOP_MODE,
+ FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR,
};
static const struct flexcan_devtype_data fsl_imx8qm_devtype_data = {
@@ -393,7 +393,7 @@ static const struct flexcan_devtype_data
fsl_imx8qm_devtype_data = {
static struct flexcan_devtype_data fsl_imx8mp_devtype_data = {
.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
FLEXCAN_QUIRK_DISABLE_MECR | FLEXCAN_QUIRK_USE_OFF_TIMESTAMP |
- FLEXCAN_QUIRK_BROKEN_PERR_STATE | FLEXCAN_QUIRK_SETUP_STOP_MODE
|
+ FLEXCAN_QUIRK_BROKEN_PERR_STATE |
FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR |
FLEXCAN_QUIRK_SUPPORT_FD | FLEXCAN_QUIRK_SUPPORT_ECC,
};
@@ -2043,7 +2043,7 @@ static int flexcan_probe(struct platform_device *pdev)
of_can_transceiver(dev);
devm_can_led_init(dev);
- if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE) {
+ if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR) {
err = flexcan_setup_stop_mode(pdev);
if (err)
dev_dbg(&pdev->dev, "failed to setup stop-mode\n");
--
2.17.1
[PATCH V3 05/10] can: flexcan: add ECC initialization for VF610
For SoCs with ECC supported, even use FLEXCAN_QUIRK_DISABLE_MECR quirk to
disable non-correctable errors interrupt and freeze mode, had better use
FLEXCAN_QUIRK_SUPPORT_ECC quirk to initialize all memory.
Fixes: cdce844865bea ("can: flexcan: add vf610 support for FlexCAN")
Signed-off-by: Joakim Zhang
---
drivers/net/can/flexcan.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index c2330eab3595..06f94b6f0ebe 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -400,7 +400,7 @@ static struct flexcan_devtype_data fsl_imx8mp_devtype_data
= {
static const struct flexcan_devtype_data fsl_vf610_devtype_data = {
.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
FLEXCAN_QUIRK_DISABLE_MECR | FLEXCAN_QUIRK_USE_OFF_TIMESTAMP |
- FLEXCAN_QUIRK_BROKEN_PERR_STATE,
+ FLEXCAN_QUIRK_BROKEN_PERR_STATE | FLEXCAN_QUIRK_SUPPORT_ECC,
};
static const struct flexcan_devtype_data fsl_ls1021a_r2_devtype_data = {
--
2.17.1
[PATCH V3 02/10] dt-bindings: can: flexcan: fix fsl,clk-source property
Correct fsl,clk-source example since flexcan driver uses "of_property_read_u8"
to get this property.
Fixes: 9d733992772d ("dt-bindings: can: flexcan: add PE clock source property
to device tree")
Signed-off-by: Joakim Zhang
---
Documentation/devicetree/bindings/net/can/fsl-flexcan.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
index e10b6eb955e1..6af67f5e581c 100644
--- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
+++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
@@ -53,5 +53,5 @@ Example:
interrupts = <48 0x2>;
interrupt-parent = <&mpic>;
clock-frequency = <2>; // filled in by bootloader
- fsl,clk-source = <0>; // select clock source 0 for PE
+ fsl,clk-source = /bits/ 8 <0>; // select clock source 0 for PE
};
--
2.17.1
[PATCH V3 10/10] can: flexcan: add CAN wakeup function for i.MX8QM
The System Controller Firmware (SCFW) is a low-level system function
which runs on a dedicated Cortex-M core to provide power, clock, and
resource management. It exists on some i.MX8 processors. e.g. i.MX8QM
(QM, QP), and i.MX8QX (QXP, DX). SCU driver manages the IPC interface
between host CPU and the SCU firmware running on M4.
For i.MX8QM, stop mode request is controlled by System Controller Unit(SCU)
firmware, this patch introduces FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW quirk
for this function.
Signed-off-by: Joakim Zhang
---
drivers/net/can/flexcan.c | 127 +-
1 file changed, 110 insertions(+), 17 deletions(-)
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 8f578c867493..97840b3e0a8a 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -9,6 +9,7 @@
//
// Based on code originally by Andrey Volkov
+#include
#include
#include
#include
@@ -17,6 +18,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -242,6 +244,8 @@
#define FLEXCAN_QUIRK_SUPPORT_FD BIT(9)
/* support memory detection and correction */
#define FLEXCAN_QUIRK_SUPPORT_ECC BIT(10)
+/* Setup stop mode with SCU firmware to support wakeup */
+#define FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW BIT(11)
/* Structure of the message buffer */
struct flexcan_mb {
@@ -347,6 +351,7 @@ struct flexcan_priv {
u8 mb_count;
u8 mb_size;
u8 clk_src; /* clock source of CAN Protocol Engine */
+ u8 can_idx;
u64 rx_mask;
u64 tx_mask;
@@ -358,6 +363,9 @@ struct flexcan_priv {
struct regulator *reg_xceiver;
struct flexcan_stop_mode stm;
+ /* IPC handle when setup stop mode by System Controller firmware(scfw)
*/
+ struct imx_sc_ipc *sc_ipc_handle;
+
/* Read and Write APIs */
u32 (*read)(void __iomem *addr);
void (*write)(u32 val, void __iomem *addr);
@@ -387,7 +395,7 @@ static const struct flexcan_devtype_data
fsl_imx6q_devtype_data = {
static const struct flexcan_devtype_data fsl_imx8qm_devtype_data = {
.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
FLEXCAN_QUIRK_USE_OFF_TIMESTAMP |
FLEXCAN_QUIRK_BROKEN_PERR_STATE |
- FLEXCAN_QUIRK_SUPPORT_FD,
+ FLEXCAN_QUIRK_SUPPORT_FD | FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW,
};
static struct flexcan_devtype_data fsl_imx8mp_devtype_data = {
@@ -546,18 +554,42 @@ static void flexcan_enable_wakeup_irq(struct flexcan_priv
*priv, bool enable)
priv->write(reg_mcr, ®s->mcr);
}
+static int flexcan_stop_mode_enable_scfw(struct flexcan_priv *priv, bool
enabled)
+{
+ u8 idx = priv->can_idx;
+ u32 rsrc_id, val;
+
+ rsrc_id = IMX_SC_R_CAN(idx);
+
+ if (enabled)
+ val = 1;
+ else
+ val = 0;
+
+ /* stop mode request via scu firmware */
+ return imx_sc_misc_set_control(priv->sc_ipc_handle, rsrc_id,
+ IMX_SC_C_IPG_STOP, val);
+}
+
static inline int flexcan_enter_stop_mode(struct flexcan_priv *priv)
{
struct flexcan_regs __iomem *regs = priv->regs;
u32 reg_mcr;
+ int ret;
reg_mcr = priv->read(®s->mcr);
reg_mcr |= FLEXCAN_MCR_SLF_WAK;
priv->write(reg_mcr, ®s->mcr);
/* enable stop request */
- regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
- 1 << priv->stm.req_bit, 1 << priv->stm.req_bit);
+ if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW) {
+ ret = flexcan_stop_mode_enable_scfw(priv, true);
+ if (ret < 0)
+ return ret;
+ } else {
+ regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
+ 1 << priv->stm.req_bit, 1 <<
priv->stm.req_bit);
+ }
return flexcan_low_power_enter_ack(priv);
}
@@ -566,10 +598,17 @@ static inline int flexcan_exit_stop_mode(struct
flexcan_priv *priv)
{
struct flexcan_regs __iomem *regs = priv->regs;
u32 reg_mcr;
+ int ret;
/* remove stop request */
- regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
- 1 << priv->stm.req_bit, 0);
+ if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW) {
+ ret = flexcan_stop_mode_enable_scfw(priv, false);
+ if (ret < 0)
+ return ret;
+ } else {
+ regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
+ 1 << priv->stm.req_bit, 0);
+ }
reg_mcr = priv->read(®s->mcr);
reg_mcr &= ~FLEXCAN_MCR_SLF_WAK;
@@ -1838,7 +1877,7 @@ static void unregister_flexcandev(struct net_device *dev)
[PATCH V3 01/10] firmware: imx: always export SCU symbols
From: Liu Ying
Always export SCU symbols for both SCU SoCs and non-SCU SoCs to avoid
build error.
Signed-off-by: Liu Ying
Signed-off-by: Peng Fan
Signed-off-by: Joakim Zhang
---
include/linux/firmware/imx/ipc.h | 15 +++
include/linux/firmware/imx/svc/misc.h | 23 +++
2 files changed, 38 insertions(+)
diff --git a/include/linux/firmware/imx/ipc.h b/include/linux/firmware/imx/ipc.h
index 891057434858..300fa253fc30 100644
--- a/include/linux/firmware/imx/ipc.h
+++ b/include/linux/firmware/imx/ipc.h
@@ -34,6 +34,7 @@ struct imx_sc_rpc_msg {
uint8_t func;
};
+#if IS_ENABLED(CONFIG_IMX_SCU)
/*
* This is an function to send an RPC message over an IPC channel.
* It is called by client-side SCFW API function shims.
@@ -55,4 +56,18 @@ int imx_scu_call_rpc(struct imx_sc_ipc *ipc, void *msg, bool
have_resp);
* @return Returns an error code (0 = success, failed if < 0)
*/
int imx_scu_get_handle(struct imx_sc_ipc **ipc);
+
+#else
+static inline int
+imx_scu_call_rpc(struct imx_sc_ipc *ipc, void *msg, bool have_resp)
+{
+ return -EIO;
+}
+
+static inline int imx_scu_get_handle(struct imx_sc_ipc **ipc)
+{
+ return -EIO;
+}
+#endif
+
#endif /* _SC_IPC_H */
diff --git a/include/linux/firmware/imx/svc/misc.h
b/include/linux/firmware/imx/svc/misc.h
index 031dd4d3c766..d255048f17de 100644
--- a/include/linux/firmware/imx/svc/misc.h
+++ b/include/linux/firmware/imx/svc/misc.h
@@ -46,6 +46,7 @@ enum imx_misc_func {
* Control Functions
*/
+#if IS_ENABLED(CONFIG_IMX_SCU)
int imx_sc_misc_set_control(struct imx_sc_ipc *ipc, u32 resource,
u8 ctrl, u32 val);
@@ -55,4 +56,26 @@ int imx_sc_misc_get_control(struct imx_sc_ipc *ipc, u32
resource,
int imx_sc_pm_cpu_start(struct imx_sc_ipc *ipc, u32 resource,
bool enable, u64 phys_addr);
+#else
+static inline int
+imx_sc_misc_set_control(struct imx_sc_ipc *ipc, u32 resource,
+ u8 ctrl, u32 val)
+{
+ return -EIO;
+}
+
+static inline int
+imx_sc_misc_get_control(struct imx_sc_ipc *ipc, u32 resource,
+ u8 ctrl, u32 *val)
+{
+ return -EIO;
+}
+
+static inline int imx_sc_pm_cpu_start(struct imx_sc_ipc *ipc, u32 resource,
+ bool enable, u64 phys_addr)
+{
+ return -EIO;
+}
+#endif
+
#endif /* _SC_MISC_API_H */
--
2.17.1
[PATCH V3 04/10] can: flexcan: add ECC initialization for LX2160A
After double check with Layerscape CAN owner (Pankaj Bansal), confirm
that LX2160A indeed supports ECC feature, so correct the feature table.
For SoCs with ECC supported, even use FLEXCAN_QUIRK_DISABLE_MECR quirk to
disable non-correctable errors interrupt and freeze mode, had better use
FLEXCAN_QUIRK_SUPPORT_ECC quirk to initialize all memory.
Fixes: 2c19bb43e5572 ("can: flexcan: add lx2160ar1 support")
Cc: Pankaj Bansal
Signed-off-by: Joakim Zhang
---
drivers/net/can/flexcan.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 586e1417a697..c2330eab3595 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -217,7 +217,7 @@
* MX8MP FlexCAN3 03.00.17.01yes yesno yes yes
yes
* VF610 FlexCAN3 ? no yesno yes
yes? no
* LS1021A FlexCAN2 03.00.04.00 no yesno no yes
no
- * LX2160A FlexCAN3 03.00.23.00 no yesno no yes
yes
+ * LX2160A FlexCAN3 03.00.23.00 no yesno yes yes
yes
*
* Some SOCs do not have the RX_WARN & TX_WARN interrupt line connected.
*/
@@ -411,7 +411,8 @@ static const struct flexcan_devtype_data
fsl_ls1021a_r2_devtype_data = {
static const struct flexcan_devtype_data fsl_lx2160a_r1_devtype_data = {
.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
FLEXCAN_QUIRK_DISABLE_MECR | FLEXCAN_QUIRK_BROKEN_PERR_STATE |
- FLEXCAN_QUIRK_USE_OFF_TIMESTAMP | FLEXCAN_QUIRK_SUPPORT_FD,
+ FLEXCAN_QUIRK_USE_OFF_TIMESTAMP | FLEXCAN_QUIRK_SUPPORT_FD |
+ FLEXCAN_QUIRK_SUPPORT_ECC,
};
static const struct can_bittiming_const flexcan_bittiming_const = {
--
2.17.1
[PATCH V3 03/10] can: flexcan: remove FLEXCAN_QUIRK_DISABLE_MECR quirk for LS1021A
After double check with Layerscape CAN owner (Pankaj Bansal), confirm that
LS1021A doesn't support ECC feature, so remove FLEXCAN_QUIRK_DISABLE_MECR
quirk.
Fixes: 99b7668c04b27 ("can: flexcan: adding platform specific details for
LS1021A")
Cc: Pankaj Bansal
Signed-off-by: Joakim Zhang
---
drivers/net/can/flexcan.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 4d594e977497..586e1417a697 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -405,8 +405,7 @@ static const struct flexcan_devtype_data
fsl_vf610_devtype_data = {
static const struct flexcan_devtype_data fsl_ls1021a_r2_devtype_data = {
.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
- FLEXCAN_QUIRK_DISABLE_MECR | FLEXCAN_QUIRK_BROKEN_PERR_STATE |
- FLEXCAN_QUIRK_USE_OFF_TIMESTAMP,
+ FLEXCAN_QUIRK_BROKEN_PERR_STATE |
FLEXCAN_QUIRK_USE_OFF_TIMESTAMP,
};
static const struct flexcan_devtype_data fsl_lx2160a_r1_devtype_data = {
--
2.17.1
[PATCH V3 00/10] can: flexcan: add stop mode support for i.MX8QM
The first patch from Liu Ying aims to export SCU symbols for SoCs w/wo SCU, so that no need to check CONFIG_IMX_SCU in the specific driver. The following patches are flexcan fixes and add stop mode support for i.MX8QM. ChangeLogs: V2->V3: * define IMX_SC_R_CAN(x) in rsrc.h * remove error message on -EPROBE_DEFER. * split disable wakeup patch into separate one. V1->V2: * split ECC fix patches into separate patches. * free can dev if failed to setup stop mode. * disable wakeup on flexcan_remove. * add FLEXCAN_IMX_SC_R_CAN macro helper. * fsl,can-index->fsl,scu-index. * move fsl,scu-index and priv->can_idx into * flexcan_setup_stop_mode_scfw() * prove failed if failed to setup stop mode. Joakim Zhang (9): dt-bindings: can: flexcan: fix fsl,clk-source property can: flexcan: remove FLEXCAN_QUIRK_DISABLE_MECR quirk for LS1021A can: flexcan: add ECC initialization for LX2160A can: flexcan: add ECC initialization for VF610 can: flexcan: disable wakeup in flexcan_remove() dt-bindings: can: flexcan: add fsl,scu-index property to indicate a resource can: flexcan: rename macro FLEXCAN_QUIRK_SETUP_STOP_MODE -> FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR dt-bindings: firmware: add IMX_SC_R_CAN(x) macro for CAN can: flexcan: add CAN wakeup function for i.MX8QM Liu Ying (1): firmware: imx: always export SCU symbols .../bindings/net/can/fsl-flexcan.txt | 8 +- drivers/net/can/flexcan.c | 147 ++ include/dt-bindings/firmware/imx/rsrc.h | 1 + include/linux/firmware/imx/ipc.h | 15 ++ include/linux/firmware/imx/svc/misc.h | 23 +++ 5 files changed, 167 insertions(+), 27 deletions(-) -- 2.17.1
[PATCH V3 07/10] dt-bindings: can: flexcan: add fsl,scu-index property to indicate a resource
For SoCs with SCU support, need setup stop mode via SCU firmware, so this property can help indicate a resource in SCU firmware. Signed-off-by: Joakim Zhang --- Documentation/devicetree/bindings/net/can/fsl-flexcan.txt | 6 ++ 1 file changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt index 6af67f5e581c..38a7da4fef3f 100644 --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt +++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt @@ -43,6 +43,11 @@ Optional properties: 0: clock source 0 (oscillator clock) 1: clock source 1 (peripheral clock) +- fsl,scu-index: The scu index of CAN instance. + For SoCs with SCU support, need setup stop mode via SCU firmware, + so this property can help indicate a resource. It supports up to + 3 CAN instances now, so the value should be 0, 1, or 2. + - wakeup-source: enable CAN remote wakeup Example: @@ -54,4 +59,5 @@ Example: interrupt-parent = <&mpic>; clock-frequency = <2>; // filled in by bootloader fsl,clk-source = /bits/ 8 <0>; // select clock source 0 for PE + fsl,scu-index = /bits/ 8 <1>; // the second CAN instance }; -- 2.17.1
[PATCH V3 09/10] dt-bindings: firmware: add IMX_SC_R_CAN(x) macro for CAN
Add IMX_SC_R_CAN(x) macro for CAN. Suggested-by: Marc Kleine-Budde Signed-off-by: Joakim Zhang --- include/dt-bindings/firmware/imx/rsrc.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/dt-bindings/firmware/imx/rsrc.h b/include/dt-bindings/firmware/imx/rsrc.h index 54278d5c1856..43885056557c 100644 --- a/include/dt-bindings/firmware/imx/rsrc.h +++ b/include/dt-bindings/firmware/imx/rsrc.h @@ -111,6 +111,7 @@ #define IMX_SC_R_CAN_0 105 #define IMX_SC_R_CAN_1 106 #define IMX_SC_R_CAN_2 107 +#define IMX_SC_R_CAN(x)(IMX_SC_R_CAN_0 + (x)) #define IMX_SC_R_DMA_1_CH0 108 #define IMX_SC_R_DMA_1_CH1 109 #define IMX_SC_R_DMA_1_CH2 110 -- 2.17.1
[PATCH V3 06/10] can: flexcan: disable wakeup in flexcan_remove()
Disable wakeup in flexcan_remove().
Fixes: de3578c198c6 ("can: flexcan: add self wakeup support")
Fixes: 915f9666421c ("can: flexcan: add support for DT property
'wakeup-source'")
Signed-off-by: Joakim Zhang
---
drivers/net/can/flexcan.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 06f94b6f0ebe..881799bd9c5e 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -2062,6 +2062,8 @@ static int flexcan_remove(struct platform_device *pdev)
{
struct net_device *dev = platform_get_drvdata(pdev);
+ device_set_wakeup_enable(&pdev->dev, false);
+ device_set_wakeup_capable(&pdev->dev, false);
unregister_flexcandev(dev);
pm_runtime_disable(&pdev->dev);
free_candev(dev);
--
2.17.1
RE: [PATCH V2 8/8] can: flexcan: add CAN wakeup function for i.MX8QM
Hi Marc, > -Original Message- > From: Marc Kleine-Budde > Sent: 2020年10月19日 16:42 > To: Joakim Zhang ; robh...@kernel.org; > shawn...@kernel.org; s.ha...@pengutronix.de > Cc: ker...@pengutronix.de; dl-linux-imx ; Ying Liu > ; linux-...@vger.kernel.org; Pankaj Bansal > ; net...@vger.kernel.org; > linux-kernel@vger.kernel.org > Subject: Re: [PATCH V2 8/8] can: flexcan: add CAN wakeup function for > i.MX8QM > > On 10/19/20 10:39 AM, Joakim Zhang wrote: > >>> +#define FLEXCAN_IMX_SC_R_CAN(x) (IMX_SC_R_CAN_0 + (x)) > >> > >> Why not move it into the appropriate svc header file? > > > > Sorry, not quite understand. Which file do you mean the appropriate > > svc header file? Is it include/dt-bindings/firmware/imx/rsrc.h? > > yes, I meant that: > > > include/dt-bindings/firmware/imx/rsrc.h:111:#define IMX_SC_R_CAN_0 > 105 As I can see in rsrc.h file, it just list each resource sequentially, and there is a note in the comments: "Note items from list should never be changed or removed (only added to at the end of the list)." So the driver author doesn't want any scu users to change these resource macro. If we only do below change for CAN, but keep other devices unchanged, It would be very strange. And I think this code change could not be accepted. There may be another consideration, now we only has 3 CAN instances, how can we handle if later SoCs have more CAN instances, and they still want to reuse this header file. This is also reason I prefer to use these defined macros directly in flexcan driver. --- a/include/dt-bindings/firmware/imx/rsrc.h +++ b/include/dt-bindings/firmware/imx/rsrc.h @@ -108,9 +108,7 @@ #define IMX_SC_R_ADC_1 102 #define IMX_SC_R_FTM_0 103 #define IMX_SC_R_FTM_1 104 -#define IMX_SC_R_CAN_0 105 -#define IMX_SC_R_CAN_1 106 -#define IMX_SC_R_CAN_2 107 +#define IMX_SC_R_CAN(x) (105 + (x)) #define IMX_SC_R_DMA_1_CH0 108 #define IMX_SC_R_DMA_1_CH1 109 #define IMX_SC_R_DMA_1_CH2 110 Add @Aisheng Dong, could above code changes can be accepted by you? Best Regards, Joakim Zhang
RE: [PATCH V2 8/8] can: flexcan: add CAN wakeup function for i.MX8QM
Hi Marc,
> -Original Message-
> From: Marc Kleine-Budde
> Sent: 2020年10月19日 16:16
> To: Joakim Zhang ; robh...@kernel.org;
> shawn...@kernel.org; s.ha...@pengutronix.de
> Cc: ker...@pengutronix.de; dl-linux-imx ; Ying Liu
> ; linux-...@vger.kernel.org; Pankaj Bansal
> ; net...@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH V2 8/8] can: flexcan: add CAN wakeup function for
> i.MX8QM
>
> On 10/19/20 5:57 PM, Joakim Zhang wrote:
> > The System Controller Firmware (SCFW) is a low-level system function
> > which runs on a dedicated Cortex-M core to provide power, clock, and
> > resource management. It exists on some i.MX8 processors. e.g. i.MX8QM
> > (QM, QP), and i.MX8QX (QXP, DX). SCU driver manages the IPC interface
> > between host CPU and the SCU firmware running on M4.
> >
> > For i.MX8QM, stop mode request is controlled by System Controller
> > Unit(SCU) firmware, this patch introduces
> > FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW quirk for this function.
> >
> > Signed-off-by: Joakim Zhang
> > ---
> > drivers/net/can/flexcan.c | 131
> > +-
> > 1 file changed, 114 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> > index c0cdee904ca7..8ad5065c918f 100644
> > --- a/drivers/net/can/flexcan.c
> > +++ b/drivers/net/can/flexcan.c
> > @@ -9,6 +9,7 @@
> > //
> > // Based on code originally by Andrey Volkov
> >
> > +#include
> > #include
> > #include
> > #include
> > @@ -17,6 +18,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -203,6 +205,8 @@
> >
> > #define FLEXCAN_TIMEOUT_US (250)
> >
> > +#define FLEXCAN_IMX_SC_R_CAN(x)(IMX_SC_R_CAN_0 + (x))
>
> Why not move it into the appropriate svc header file?
Sorry, not quite understand. Which file do you mean the appropriate svc header
file? Is it include/dt-bindings/firmware/imx/rsrc.h?
After glancing the header file under include/linux/firmware/imx, have not found
the appropriate svc header. Could you please explain more?
> > +
> > /* FLEXCAN hardware feature flags
> > *
> > * Below is some version info we got:
> > @@ -242,6 +246,8 @@
> > #define FLEXCAN_QUIRK_SUPPORT_FD BIT(9)
> > /* support memory detection and correction */ #define
> > FLEXCAN_QUIRK_SUPPORT_ECC BIT(10)
> > +/* Setup stop mode with SCU firmware to support wakeup */ #define
> > +FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW BIT(11)
> >
> > /* Structure of the message buffer */ struct flexcan_mb { @@ -347,6
> > +353,7 @@ struct flexcan_priv {
> > u8 mb_count;
> > u8 mb_size;
> > u8 clk_src; /* clock source of CAN Protocol Engine */
> > + u8 can_idx;
> >
> > u64 rx_mask;
> > u64 tx_mask;
> > @@ -358,6 +365,9 @@ struct flexcan_priv {
> > struct regulator *reg_xceiver;
> > struct flexcan_stop_mode stm;
> >
> > + /* IPC handle when setup stop mode by System Controller firmware(scfw)
> */
> > + struct imx_sc_ipc *sc_ipc_handle;
> > +
> > /* Read and Write APIs */
> > u32 (*read)(void __iomem *addr);
> > void (*write)(u32 val, void __iomem *addr); @@ -387,7 +397,7 @@
> > static const struct flexcan_devtype_data fsl_imx6q_devtype_data = {
> > static const struct flexcan_devtype_data fsl_imx8qm_devtype_data = {
> > .quirks = FLEXCAN_QUIRK_DISABLE_RXFG |
> FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
> > FLEXCAN_QUIRK_USE_OFF_TIMESTAMP |
> FLEXCAN_QUIRK_BROKEN_PERR_STATE |
> > - FLEXCAN_QUIRK_SUPPORT_FD,
> > + FLEXCAN_QUIRK_SUPPORT_FD |
> FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW,
> > };
> >
> > static struct flexcan_devtype_data fsl_imx8mp_devtype_data = { @@
> > -546,18 +556,42 @@ static void flexcan_enable_wakeup_irq(struct
> flexcan_priv *priv, bool enable)
> > priv->write(reg_mcr, ®s->mcr);
> > }
> >
> > +static int flexcan_stop_mode_enable_scfw(struct flexcan_priv *priv,
> > +bool enabled) {
> > + u8 idx = priv->can_idx;
> > + u32 rsrc_id, val;
> > +
> > + rsrc_id = FLEXCAN_IMX_SC_R_CAN(idx);
> > +
> > + if (enabled)
> > + val = 1;
> > + else
> > + val = 0;
> > +
> > + /* stop mode request via scu firmware */
> > + return imx_sc_misc_set_control(priv->sc_ipc_handle, rsrc_id,
> > +
[PATCH V2 5/8] can: flexcan: add ECC initialization for VF610
For SoCs with ECC supported, even use FLEXCAN_QUIRK_DISABLE_MECR quirk to
disable non-correctable errors interrupt and freeze mode, had better use
FLEXCAN_QUIRK_SUPPORT_ECC quirk to initialize all memory.
Fixes: cdce844865bea ("can: flexcan: add vf610 support for FlexCAN")
Signed-off-by: Joakim Zhang
---
drivers/net/can/flexcan.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index c2330eab3595..06f94b6f0ebe 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -400,7 +400,7 @@ static struct flexcan_devtype_data fsl_imx8mp_devtype_data
= {
static const struct flexcan_devtype_data fsl_vf610_devtype_data = {
.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
FLEXCAN_QUIRK_DISABLE_MECR | FLEXCAN_QUIRK_USE_OFF_TIMESTAMP |
- FLEXCAN_QUIRK_BROKEN_PERR_STATE,
+ FLEXCAN_QUIRK_BROKEN_PERR_STATE | FLEXCAN_QUIRK_SUPPORT_ECC,
};
static const struct flexcan_devtype_data fsl_ls1021a_r2_devtype_data = {
--
2.17.1
[PATCH V2 8/8] can: flexcan: add CAN wakeup function for i.MX8QM
The System Controller Firmware (SCFW) is a low-level system function
which runs on a dedicated Cortex-M core to provide power, clock, and
resource management. It exists on some i.MX8 processors. e.g. i.MX8QM
(QM, QP), and i.MX8QX (QXP, DX). SCU driver manages the IPC interface
between host CPU and the SCU firmware running on M4.
For i.MX8QM, stop mode request is controlled by System Controller Unit(SCU)
firmware, this patch introduces FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW quirk
for this function.
Signed-off-by: Joakim Zhang
---
drivers/net/can/flexcan.c | 131 +-
1 file changed, 114 insertions(+), 17 deletions(-)
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index c0cdee904ca7..8ad5065c918f 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -9,6 +9,7 @@
//
// Based on code originally by Andrey Volkov
+#include
#include
#include
#include
@@ -17,6 +18,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -203,6 +205,8 @@
#define FLEXCAN_TIMEOUT_US (250)
+#define FLEXCAN_IMX_SC_R_CAN(x)(IMX_SC_R_CAN_0 + (x))
+
/* FLEXCAN hardware feature flags
*
* Below is some version info we got:
@@ -242,6 +246,8 @@
#define FLEXCAN_QUIRK_SUPPORT_FD BIT(9)
/* support memory detection and correction */
#define FLEXCAN_QUIRK_SUPPORT_ECC BIT(10)
+/* Setup stop mode with SCU firmware to support wakeup */
+#define FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW BIT(11)
/* Structure of the message buffer */
struct flexcan_mb {
@@ -347,6 +353,7 @@ struct flexcan_priv {
u8 mb_count;
u8 mb_size;
u8 clk_src; /* clock source of CAN Protocol Engine */
+ u8 can_idx;
u64 rx_mask;
u64 tx_mask;
@@ -358,6 +365,9 @@ struct flexcan_priv {
struct regulator *reg_xceiver;
struct flexcan_stop_mode stm;
+ /* IPC handle when setup stop mode by System Controller firmware(scfw)
*/
+ struct imx_sc_ipc *sc_ipc_handle;
+
/* Read and Write APIs */
u32 (*read)(void __iomem *addr);
void (*write)(u32 val, void __iomem *addr);
@@ -387,7 +397,7 @@ static const struct flexcan_devtype_data
fsl_imx6q_devtype_data = {
static const struct flexcan_devtype_data fsl_imx8qm_devtype_data = {
.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
FLEXCAN_QUIRK_USE_OFF_TIMESTAMP |
FLEXCAN_QUIRK_BROKEN_PERR_STATE |
- FLEXCAN_QUIRK_SUPPORT_FD,
+ FLEXCAN_QUIRK_SUPPORT_FD | FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW,
};
static struct flexcan_devtype_data fsl_imx8mp_devtype_data = {
@@ -546,18 +556,42 @@ static void flexcan_enable_wakeup_irq(struct flexcan_priv
*priv, bool enable)
priv->write(reg_mcr, ®s->mcr);
}
+static int flexcan_stop_mode_enable_scfw(struct flexcan_priv *priv, bool
enabled)
+{
+ u8 idx = priv->can_idx;
+ u32 rsrc_id, val;
+
+ rsrc_id = FLEXCAN_IMX_SC_R_CAN(idx);
+
+ if (enabled)
+ val = 1;
+ else
+ val = 0;
+
+ /* stop mode request via scu firmware */
+ return imx_sc_misc_set_control(priv->sc_ipc_handle, rsrc_id,
+ IMX_SC_C_IPG_STOP, val);
+}
+
static inline int flexcan_enter_stop_mode(struct flexcan_priv *priv)
{
struct flexcan_regs __iomem *regs = priv->regs;
u32 reg_mcr;
+ int ret;
reg_mcr = priv->read(®s->mcr);
reg_mcr |= FLEXCAN_MCR_SLF_WAK;
priv->write(reg_mcr, ®s->mcr);
/* enable stop request */
- regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
- 1 << priv->stm.req_bit, 1 << priv->stm.req_bit);
+ if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW) {
+ ret = flexcan_stop_mode_enable_scfw(priv, true);
+ if (ret < 0)
+ return ret;
+ } else {
+ regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
+ 1 << priv->stm.req_bit, 1 <<
priv->stm.req_bit);
+ }
return flexcan_low_power_enter_ack(priv);
}
@@ -566,10 +600,17 @@ static inline int flexcan_exit_stop_mode(struct
flexcan_priv *priv)
{
struct flexcan_regs __iomem *regs = priv->regs;
u32 reg_mcr;
+ int ret;
/* remove stop request */
- regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
- 1 << priv->stm.req_bit, 0);
+ if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_SCFW) {
+ ret = flexcan_stop_mode_enable_scfw(priv, false);
+ if (ret < 0)
+ return ret;
+ } else {
+ regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
+
[PATCH V2 6/8] dt-bindings: can: flexcan: add fsl,scu-index property to indicate a resource
For SoCs with SCU support, need setup stop mode via SCU firmware, so this property can help indicate a resource in SCU firmware. Signed-off-by: Joakim Zhang --- Documentation/devicetree/bindings/net/can/fsl-flexcan.txt | 6 ++ 1 file changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt index 6af67f5e581c..38a7da4fef3f 100644 --- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt +++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt @@ -43,6 +43,11 @@ Optional properties: 0: clock source 0 (oscillator clock) 1: clock source 1 (peripheral clock) +- fsl,scu-index: The scu index of CAN instance. + For SoCs with SCU support, need setup stop mode via SCU firmware, + so this property can help indicate a resource. It supports up to + 3 CAN instances now, so the value should be 0, 1, or 2. + - wakeup-source: enable CAN remote wakeup Example: @@ -54,4 +59,5 @@ Example: interrupt-parent = <&mpic>; clock-frequency = <2>; // filled in by bootloader fsl,clk-source = /bits/ 8 <0>; // select clock source 0 for PE + fsl,scu-index = /bits/ 8 <1>; // the second CAN instance }; -- 2.17.1
[PATCH V2 7/8] can: flexcan: rename macro FLEXCAN_QUIRK_SETUP_STOP_MODE -> FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR
This patch intends to rename FLEXCAN_QUIRK_SETUP_STOP_MODE quirk
to FLEXCAN_QUIRK_SETUP_STOP_MODE_GRP for non-scu SoCs, coming patch will
add quirk for scu SoCs.
For non-scu SoCs, setup stop mode with GPR register.
For scu SoCs, setup stop mode with SCU firmware.
Signed-off-by: Joakim Zhang
---
drivers/net/can/flexcan.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 06f94b6f0ebe..c0cdee904ca7 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -236,8 +236,8 @@
#define FLEXCAN_QUIRK_BROKEN_PERR_STATE BIT(6)
/* default to BE register access */
#define FLEXCAN_QUIRK_DEFAULT_BIG_ENDIAN BIT(7)
-/* Setup stop mode to support wakeup */
-#define FLEXCAN_QUIRK_SETUP_STOP_MODE BIT(8)
+/* Setup stop mode with GPR to support wakeup */
+#define FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR BIT(8)
/* Support CAN-FD mode */
#define FLEXCAN_QUIRK_SUPPORT_FD BIT(9)
/* support memory detection and correction */
@@ -381,7 +381,7 @@ static const struct flexcan_devtype_data
fsl_imx28_devtype_data = {
static const struct flexcan_devtype_data fsl_imx6q_devtype_data = {
.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
FLEXCAN_QUIRK_USE_OFF_TIMESTAMP |
FLEXCAN_QUIRK_BROKEN_PERR_STATE |
- FLEXCAN_QUIRK_SETUP_STOP_MODE,
+ FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR,
};
static const struct flexcan_devtype_data fsl_imx8qm_devtype_data = {
@@ -393,7 +393,7 @@ static const struct flexcan_devtype_data
fsl_imx8qm_devtype_data = {
static struct flexcan_devtype_data fsl_imx8mp_devtype_data = {
.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
FLEXCAN_QUIRK_DISABLE_MECR | FLEXCAN_QUIRK_USE_OFF_TIMESTAMP |
- FLEXCAN_QUIRK_BROKEN_PERR_STATE | FLEXCAN_QUIRK_SETUP_STOP_MODE
|
+ FLEXCAN_QUIRK_BROKEN_PERR_STATE |
FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR |
FLEXCAN_QUIRK_SUPPORT_FD | FLEXCAN_QUIRK_SUPPORT_ECC,
};
@@ -2043,7 +2043,7 @@ static int flexcan_probe(struct platform_device *pdev)
of_can_transceiver(dev);
devm_can_led_init(dev);
- if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE) {
+ if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR) {
err = flexcan_setup_stop_mode(pdev);
if (err)
dev_dbg(&pdev->dev, "failed to setup stop-mode\n");
--
2.17.1
[PATCH V2 4/8] can: flexcan: add ECC initialization for LX2160A
After double check with Layerscape CAN owner (Pankaj Bansal), confirm
that LX2160A indeed supports ECC feature, so correct the feature table.
For SoCs with ECC supported, even use FLEXCAN_QUIRK_DISABLE_MECR quirk to
disable non-correctable errors interrupt and freeze mode, had better use
FLEXCAN_QUIRK_SUPPORT_ECC quirk to initialize all memory.
Fixes: 2c19bb43e5572 ("can: flexcan: add lx2160ar1 support")
Cc: Pankaj Bansal
Signed-off-by: Joakim Zhang
---
drivers/net/can/flexcan.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 586e1417a697..c2330eab3595 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -217,7 +217,7 @@
* MX8MP FlexCAN3 03.00.17.01yes yesno yes yes
yes
* VF610 FlexCAN3 ? no yesno yes
yes? no
* LS1021A FlexCAN2 03.00.04.00 no yesno no yes
no
- * LX2160A FlexCAN3 03.00.23.00 no yesno no yes
yes
+ * LX2160A FlexCAN3 03.00.23.00 no yesno yes yes
yes
*
* Some SOCs do not have the RX_WARN & TX_WARN interrupt line connected.
*/
@@ -411,7 +411,8 @@ static const struct flexcan_devtype_data
fsl_ls1021a_r2_devtype_data = {
static const struct flexcan_devtype_data fsl_lx2160a_r1_devtype_data = {
.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
FLEXCAN_QUIRK_DISABLE_MECR | FLEXCAN_QUIRK_BROKEN_PERR_STATE |
- FLEXCAN_QUIRK_USE_OFF_TIMESTAMP | FLEXCAN_QUIRK_SUPPORT_FD,
+ FLEXCAN_QUIRK_USE_OFF_TIMESTAMP | FLEXCAN_QUIRK_SUPPORT_FD |
+ FLEXCAN_QUIRK_SUPPORT_ECC,
};
static const struct can_bittiming_const flexcan_bittiming_const = {
--
2.17.1
[PATCH V2 1/8] firmware: imx: always export SCU symbols
From: Liu Ying
Always export SCU symbols for both SCU SoCs and non-SCU SoCs to avoid
build error.
Signed-off-by: Liu Ying
Signed-off-by: Peng Fan
Signed-off-by: Joakim Zhang
---
include/linux/firmware/imx/ipc.h | 15 +++
include/linux/firmware/imx/svc/misc.h | 23 +++
2 files changed, 38 insertions(+)
diff --git a/include/linux/firmware/imx/ipc.h b/include/linux/firmware/imx/ipc.h
index 891057434858..300fa253fc30 100644
--- a/include/linux/firmware/imx/ipc.h
+++ b/include/linux/firmware/imx/ipc.h
@@ -34,6 +34,7 @@ struct imx_sc_rpc_msg {
uint8_t func;
};
+#if IS_ENABLED(CONFIG_IMX_SCU)
/*
* This is an function to send an RPC message over an IPC channel.
* It is called by client-side SCFW API function shims.
@@ -55,4 +56,18 @@ int imx_scu_call_rpc(struct imx_sc_ipc *ipc, void *msg, bool
have_resp);
* @return Returns an error code (0 = success, failed if < 0)
*/
int imx_scu_get_handle(struct imx_sc_ipc **ipc);
+
+#else
+static inline int
+imx_scu_call_rpc(struct imx_sc_ipc *ipc, void *msg, bool have_resp)
+{
+ return -EIO;
+}
+
+static inline int imx_scu_get_handle(struct imx_sc_ipc **ipc)
+{
+ return -EIO;
+}
+#endif
+
#endif /* _SC_IPC_H */
diff --git a/include/linux/firmware/imx/svc/misc.h
b/include/linux/firmware/imx/svc/misc.h
index 031dd4d3c766..d255048f17de 100644
--- a/include/linux/firmware/imx/svc/misc.h
+++ b/include/linux/firmware/imx/svc/misc.h
@@ -46,6 +46,7 @@ enum imx_misc_func {
* Control Functions
*/
+#if IS_ENABLED(CONFIG_IMX_SCU)
int imx_sc_misc_set_control(struct imx_sc_ipc *ipc, u32 resource,
u8 ctrl, u32 val);
@@ -55,4 +56,26 @@ int imx_sc_misc_get_control(struct imx_sc_ipc *ipc, u32
resource,
int imx_sc_pm_cpu_start(struct imx_sc_ipc *ipc, u32 resource,
bool enable, u64 phys_addr);
+#else
+static inline int
+imx_sc_misc_set_control(struct imx_sc_ipc *ipc, u32 resource,
+ u8 ctrl, u32 val)
+{
+ return -EIO;
+}
+
+static inline int
+imx_sc_misc_get_control(struct imx_sc_ipc *ipc, u32 resource,
+ u8 ctrl, u32 *val)
+{
+ return -EIO;
+}
+
+static inline int imx_sc_pm_cpu_start(struct imx_sc_ipc *ipc, u32 resource,
+ bool enable, u64 phys_addr)
+{
+ return -EIO;
+}
+#endif
+
#endif /* _SC_MISC_API_H */
--
2.17.1
[PATCH V2 0/8] can: flexcan: add stop mode support for i.MX8QM
The first patch from Liu Ying aims to export SCU symbols for SoCs w/wo SCU, so that no need to check CONFIG_IMX_SCU in the specific driver. The following patches are flexcan fixes and add stop mode support for i.MX8QM. ChangeLogs: V1->V2: * split ECC fix patches into separate patches. * free can dev if failed to setup stop mode. * disable wakeup on flexcan_remove. * add FLEXCAN_IMX_SC_R_CAN macro helper. * fsl,can-index->fsl,scu-index. * move fsl,scu-index and priv->can_idx into * flexcan_setup_stop_mode_scfw() * prove failed if failed to setup stop mode. Joakim Zhang (7): dt-bindings: can: flexcan: fix fsl,clk-source property can: flexcan: remove FLEXCAN_QUIRK_DISABLE_MECR quirk for LS1021A can: flexcan: add ECC initialization for LX2160A can: flexcan: add ECC initialization for VF610 dt-bindings: can: flexcan: add fsl,scu-index property to indicate a resource can: flexcan: rename macro FLEXCAN_QUIRK_SETUP_STOP_MODE -> FLEXCAN_QUIRK_SETUP_STOP_MODE_GPR can: flexcan: add CAN wakeup function for i.MX8QM Liu Ying (1): firmware: imx: always export SCU symbols .../bindings/net/can/fsl-flexcan.txt | 8 +- drivers/net/can/flexcan.c | 149 +++--- include/linux/firmware/imx/ipc.h | 15 ++ include/linux/firmware/imx/svc/misc.h | 23 +++ 4 files changed, 168 insertions(+), 27 deletions(-) -- 2.17.1
