[PATCH] ptp: Don't print an error if ptp_kvm is not supported

[Jon Hunter]

Commit 300bb1fe7671 ("ptp: arm/arm64: Enable ptp_kvm for arm/arm64")
enable ptp_kvm support for ARM platforms and for any ARM platform that
does not support this, the following error message is displayed ...

 ERR KERN fail to initialize ptp_kvm

For platforms that do not support ptp_kvm this error is a bit misleading
and so fix this by only printing this message if the error returned by
kvm_arch_ptp_init() is not -EOPNOTSUPP. Note that -EOPNOTSUPP is only
returned by ARM platforms today if ptp_kvm is not supported.

Fixes: 300bb1fe7671 ("ptp: arm/arm64: Enable ptp_kvm for arm/arm64")
Signed-off-by: Jon Hunter 
---
 drivers/ptp/ptp_kvm_common.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/ptp/ptp_kvm_common.c b/drivers/ptp/ptp_kvm_common.c
index 721ddcede5e1..fcae32f56f25 100644
--- a/drivers/ptp/ptp_kvm_common.c
+++ b/drivers/ptp/ptp_kvm_common.c
@@ -138,7 +138,8 @@ static int __init ptp_kvm_init(void)
 
ret = kvm_arch_ptp_init();
if (ret) {
-   pr_err("fail to initialize ptp_kvm");
+   if (ret != -EOPNOTSUPP)
+   pr_err("fail to initialize ptp_kvm");
return ret;
}
 
-- 
2.25.1

Re: [PATCH v3 01/14] PCI: tegra: Convert to MSI domains

[Jon Hunter]

Hi Marc,

On 20/04/2021 09:39, Marc Zyngier wrote:

...

> The following should hopefully cure it (compile tested only). Please
> let me know.
> 
>   M.
> 
> diff --git a/drivers/pci/controller/pci-tegra.c 
> b/drivers/pci/controller/pci-tegra.c
> index eaba7b2fab4a..507b23d43ad1 100644
> --- a/drivers/pci/controller/pci-tegra.c
> +++ b/drivers/pci/controller/pci-tegra.c
> @@ -1802,13 +1802,19 @@ static void tegra_pcie_enable_msi(struct tegra_pcie 
> *pcie)
>  {
>   const struct tegra_pcie_soc *soc = pcie->soc;
>   struct tegra_msi *msi = >msi;
> - u32 reg;
> + u32 reg, msi_state[INT_PCI_MSI_NR / 32];
> + int i;
>  
>   afi_writel(pcie, msi->phys >> soc->msi_base_shift, AFI_MSI_FPCI_BAR_ST);
>   afi_writel(pcie, msi->phys, AFI_MSI_AXI_BAR_ST);
>   /* this register is in 4K increments */
>   afi_writel(pcie, 1, AFI_MSI_BAR_SZ);
>  
> + /* Restore the MSI allocation state */
> + bitmap_to_arr32(msi_state, msi->used, INT_PCI_MSI_NR);
> + for (i = 0; i < ARRAY_SIZE(msi_state); i++)
> + afi_writel(pcie, msi_state[i], AFI_MSI_EN_VEC(i));
> +
>   /* and unmask the MSI interrupt */
>   reg = afi_readl(pcie, AFI_INTR_MASK);
>   reg |= AFI_INTR_MASK_MSI_MASK;
> 


Thanks, that does fix it indeed!

Tested-by: Jon Hunter 

Cheers
Jon

-- 
nvpublic

Re: [PATCH v3 1/3] tracing: Show real address for trace event arguments

[Jon Hunter]

On 19/04/2021 19:22, Steven Rostedt wrote:
> On Mon, 19 Apr 2021 14:08:14 +0100
> Jon Hunter  wrote:
> 
>> I have encountered the following crash on a couple of our ARM64 Jetson
>> platforms and bisect is pointing to this change. The crash I am seeing
>> is on boot when I am directing the trace prints to the console by adding
>> 'tp_printk trace_event="cpu_frequency,cpu_frequency_limits"' to the
>> kernel command line and enabling CONFIG_BOOTTIME_TRACING. Reverting this
>> change does fix the problem. Let me know if you have any thoughts.
> 
> Thanks for the report. I was able to reproduce this on x86 as well.
> 
> It's the tp_printk that's the problem. Does this fix it for you?
> 
> -- Steve
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 66a4ad93b5e9..f1ce4be7a499 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -3580,7 +3580,11 @@ static char *trace_iter_expand_format(struct 
> trace_iterator *iter)
>  {
>   char *tmp;
>  
> - if (iter->fmt == static_fmt_buf)
> + /*
> +  * iter->tr is NULL when used with tp_printk, which makes
> +  * this get called where it is not safe to call krealloc().
> +  */
> + if (!iter->tr || iter->fmt == static_fmt_buf)
>   return NULL;
>  
>   tmp = krealloc(iter->fmt, iter->fmt_size + STATIC_FMT_BUF_SIZE,
> @@ -3799,7 +3803,7 @@ const char *trace_event_format(struct trace_iterator 
> *iter, const char *fmt)
>   if (WARN_ON_ONCE(!fmt))
>   return fmt;
>  
> - if (iter->tr->trace_flags & TRACE_ITER_HASH_PTR)
> + if (!iter->tr || iter->tr->trace_flags & TRACE_ITER_HASH_PTR)
>   return fmt;
>  
>   p = fmt;
> @@ -9931,7 +9935,7 @@ void __init early_trace_init(void)
>  {
>   if (tracepoint_printk) {
>   tracepoint_print_iter =
> - kmalloc(sizeof(*tracepoint_print_iter), GFP_KERNEL);
> + kzalloc(sizeof(*tracepoint_print_iter), GFP_KERNEL);
>   if (MEM_FAIL(!tracepoint_print_iter,
>"Failed to allocate trace iterator\n"))
>   tracepoint_printk = 0;
> 


Yes that works for me thanks!

Tested-by: Jon Hunter 

Cheers!
Jon

-- 
nvpublic

Re: [PATCH v3 01/14] PCI: tegra: Convert to MSI domains

[Jon Hunter]

On 19/04/2021 20:19, Jon Hunter wrote:
> Hi Marc,
> 
> On 30/03/2021 16:11, Marc Zyngier wrote:
>> In anticipation of the removal of the msi_controller structure, convert
>> the Tegra host controller driver to MSI domains.
>>
>> We end-up with the usual two domain structure, the top one being a
>> generic PCI/MSI domain, the bottom one being Tegra-specific and handling
>> the actual HW interrupt allocation.
>>
>> While at it, convert the normal interrupt handler to a chained handler,
>> handle the controller's MSI IRQ edge triggered, support multiple MSIs
>> per device and use the AFI_MSI_EN_VEC* registers to provide MSI masking.
>>
>> Acked-by: Bjorn Helgaas 
>> [tred...@nvidia.com: fix, clean up and address TODOs from Marc's draft]
>> Signed-off-by: Thierry Reding 
>> Signed-off-by: Marc Zyngier 
> 
> 
> This change is breaking a suspend test that we are running on Tegra124
> Jetson-TK1. The Tegra124 Jetson TK1 uses a PCI based ethernet device ...
> 
> $ lspci
> 00:02.0 PCI bridge: NVIDIA Corporation TegraK1 PCIe x1 Bridge (rev a1)
> 01:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd.
> RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller (rev 0c)
> 
> After resuming from suspend, networking is no longer working. The reason
> why this breaks our suspend test is because that setup is using NFS for
> the rootfs. I am looking into it, but if anyone has any thoughts please
> let me know.


So the following does appear to fix it ...

diff --git a/drivers/pci/controller/pci-tegra.c
b/drivers/pci/controller/pci-tegra.c
index eaba7b2fab4a..558f02e0693d 100644
--- a/drivers/pci/controller/pci-tegra.c
+++ b/drivers/pci/controller/pci-tegra.c
@@ -1802,13 +1802,17 @@ static void tegra_pcie_enable_msi(struct
tegra_pcie *pcie)
 {
const struct tegra_pcie_soc *soc = pcie->soc;
struct tegra_msi *msi = >msi;
-   u32 reg;
+   u32 i, reg;

afi_writel(pcie, msi->phys >> soc->msi_base_shift,
AFI_MSI_FPCI_BAR_ST);
afi_writel(pcie, msi->phys, AFI_MSI_AXI_BAR_ST);
/* this register is in 4K increments */
afi_writel(pcie, 1, AFI_MSI_BAR_SZ);

+   /* enable all MSI vectors */
+   for (i = 0; i < 8; i++)
+   afi_writel(pcie, 0x, AFI_MSI_EN_VEC(i));
+
/* and unmask the MSI interrupt */
reg = afi_readl(pcie, AFI_INTR_MASK);
reg |= AFI_INTR_MASK_MSI_MASK;
@@ -1837,13 +1841,17 @@ static void tegra_pcie_msi_teardown(struct
tegra_pcie *pcie)

 static int tegra_pcie_disable_msi(struct tegra_pcie *pcie)
 {
-   u32 value;
+   u32 i, value;

/* mask the MSI interrupt */
value = afi_readl(pcie, AFI_INTR_MASK);
value &= ~AFI_INTR_MASK_MSI_MASK;
afi_writel(pcie, value, AFI_INTR_MASK);

+   /* disable all MSI vectors */
+   for (i = 0; i < 8; i++)
+   afi_writel(pcie, 0, AFI_MSI_EN_VEC(i));
+
return 0;
 }


Any reason why that code was removed?

Thanks
Jon

-- 
nvpublic

Re: [PATCH v3 01/14] PCI: tegra: Convert to MSI domains

[Jon Hunter]

Hi Marc,

On 30/03/2021 16:11, Marc Zyngier wrote:
> In anticipation of the removal of the msi_controller structure, convert
> the Tegra host controller driver to MSI domains.
> 
> We end-up with the usual two domain structure, the top one being a
> generic PCI/MSI domain, the bottom one being Tegra-specific and handling
> the actual HW interrupt allocation.
> 
> While at it, convert the normal interrupt handler to a chained handler,
> handle the controller's MSI IRQ edge triggered, support multiple MSIs
> per device and use the AFI_MSI_EN_VEC* registers to provide MSI masking.
> 
> Acked-by: Bjorn Helgaas 
> [tred...@nvidia.com: fix, clean up and address TODOs from Marc's draft]
> Signed-off-by: Thierry Reding 
> Signed-off-by: Marc Zyngier 


This change is breaking a suspend test that we are running on Tegra124
Jetson-TK1. The Tegra124 Jetson TK1 uses a PCI based ethernet device ...

$ lspci
00:02.0 PCI bridge: NVIDIA Corporation TegraK1 PCIe x1 Bridge (rev a1)
01:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd.
RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller (rev 0c)

After resuming from suspend, networking is no longer working. The reason
why this breaks our suspend test is because that setup is using NFS for
the rootfs. I am looking into it, but if anyone has any thoughts please
let me know.

Jon

-- 
nvpublic

Re: [PATCH v3 1/3] tracing: Show real address for trace event arguments

[Jon Hunter]

On 15/10/2020 15:55, Masami Hiramatsu wrote:
> To help debugging kernel, show real address for trace event arguments
> in tracefs/trace{,pipe} instead of hashed pointer value.
> 
> Since ftrace human-readable format uses vsprintf(), all %p are
> translated to hash values instead of pointer address.
> 
> However, when debugging the kernel, raw address value gives a
> hint when comparing with the memory mapping in the kernel.
> (Those are sometimes used with crash log, which is not hashed too)
> So converting %p with %px when calling trace_seq_printf().
> 
> Moreover, this is not improving the security because the tracefs
> can be used only by root user and the raw address values are readable
> from tracefs/percpu/cpu*/trace_pipe_raw file.
> 
> Signed-off-by: Masami Hiramatsu 


I have encountered the following crash on a couple of our ARM64 Jetson
platforms and bisect is pointing to this change. The crash I am seeing
is on boot when I am directing the trace prints to the console by adding
'tp_printk trace_event="cpu_frequency,cpu_frequency_limits"' to the
kernel command line and enabling CONFIG_BOOTTIME_TRACING. Reverting this
change does fix the problem. Let me know if you have any thoughts.

[5.731301] Unable to handle kernel paging request at virtual address
fbf8800040f8
[5.739215] Mem abort info:
[5.742004]   ESR = 0x9604
[5.745054]   EC = 0x25: DABT (current EL), IL = 32 bits
[5.750359]   SET = 0, FnV = 0
[5.753408]   EA = 0, S1PTW = 0
[5.756543] Data abort info:
[5.759416]   ISV = 0, ISS = 0x0004
[5.763244]   CM = 0, WnR = 0
[5.766205] [fbf8800040f8] address between user and kernel
address ranges
[5.773332] Internal error: Oops: 9604 [#1] PREEMPT SMP
[5.778898] Modules linked in:
[5.781952] CPU: 5 PID: 44 Comm: kworker/5:1 Tainted: G S
5.12.0-rc8 #1
[5.789861] Hardware name: NVIDIA Jetson TX2 Developer Kit (DT)
[5.795773] Workqueue: events deferred_probe_work_func
[5.800913] pstate: 6085 (nZCv daIf -PAN -UAO -TCO BTYPE=--)
[5.806911] pc : trace_event_format+0x28/0x1a0
[5.811353] lr : trace_event_printf+0x50/0x98
[5.815706] sp : 80001230b760
[5.819014] x29: 80001230b760 x28: 800011e99c10
[5.824324] x27: 82d9ce18 x26: 8000115abd30
[5.829630] x25:  x24: 800011f3e040
[5.834937] x23: 800050c8 x22: 80004000
[5.840242] x21: 800012276e38 x20: 80004000
[5.845549] x19: 800011e99948 x18: 
[5.850854] x17: 0007 x16: 0001
[5.856161] x15: 800011e99948 x14: 82d15688
[5.861468] x13: 82d15687 x12: 0018
[5.866774] x11: 0101010101010101 x10: 7f7f7f7f7f7f7f7f
[5.872082] x9 : 0090 x8 : 0095
[5.877389] x7 : 0014 x6 : 800050de
[5.882695] x5 :  x4 : 
[5.888002] x3 : 80001230b820 x2 : ffd0
[5.893308] x1 : 8000115abd30 x0 : fbf880004080
[5.898615] Call trace:
[5.901056]  trace_event_format+0x28/0x1a0
[5.905147]  trace_event_printf+0x50/0x98
[5.909151]  trace_raw_output_cpu_frequency_limits+0x48/0x60
[5.914807]  trace_event_buffer_commit+0x1bc/0x288
[5.919592]  trace_event_raw_event_cpu_frequency_limits+0x78/0xd0
[5.925679]  __traceiter_cpu_frequency_limits+0x30/0x48
[5.930899]  cpufreq_set_policy+0x15c/0x288
[5.935079]  cpufreq_online+0x6f4/0x928
[5.938910]  cpufreq_add_dev+0x78/0x88
[5.942654]  subsys_interface_register+0x9c/0xf0
[5.947266]  cpufreq_register_driver+0x170/0x218
[5.951878]  tegra186_cpufreq_probe+0x164/0x350
[5.956404]  platform_probe+0x90/0xd8
[5.960061]  really_probe+0x20c/0x3e8
[5.963720]  driver_probe_device+0x54/0xb8
[5.967812]  __device_attach_driver+0x90/0xc0
[5.972161]  bus_for_each_drv+0x70/0xc8
[5.975992]  __device_attach+0xec/0x150
[5.979825]  device_initial_probe+0x10/0x18
[5.984002]  bus_probe_device+0x94/0xa0
[5.987833]  deferred_probe_work_func+0x80/0xb8
[5.992359]  process_one_work+0x1f0/0x4b8
[5.996368]  worker_thread+0x20c/0x450
[6.000112]  kthread+0x124/0x150
[6.003337]  ret_from_fork+0x10/0x18
[6.006913] Code: b4000b21 aa0003f6 f940 aa0103fa (b9407800)
[6.013000] ---[ end trace eae1531f47c7c14a ]---

Thanks!
Jon

-- 
nvpublic

Re: Regression v5.12-rc3: net: stmmac: re-init rx buffers when mac resume back

[Jon Hunter]

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?

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(>lock);
+   stmmac_reinit_rx_buffers(priv);
+   mutex_unlock(>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(>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 sufficient to fix your problem.

How often does the issue you see occur?

Thanks
Jon

-- 
nvpublic

Re: [PATCH] PCI: tegra: Fix runtime PM imbalance in pex_ep_event_pex_rst_deassert

[Jon Hunter]

On 08/04/2021 08:26, Dinghao Liu wrote:
> pm_runtime_get_sync() will increase the runtime PM counter
> even it returns an error. Thus a pairing decrement is needed
> to prevent refcount leak. Fix this by replacing this API with
> pm_runtime_resume_and_get(), which will not change the runtime
> PM counter on error.
> 
> Signed-off-by: Dinghao Liu 
> ---
>  drivers/pci/controller/dwc/pcie-tegra194.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c 
> b/drivers/pci/controller/dwc/pcie-tegra194.c
> index 6fa216e52d14..0e94190ca4e8 100644
> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> @@ -1645,7 +1645,7 @@ static void pex_ep_event_pex_rst_deassert(struct 
> tegra_pcie_dw *pcie)
>   if (pcie->ep_state == EP_STATE_ENABLED)
>   return;
>  
> - ret = pm_runtime_get_sync(dev);
> + ret = pm_runtime_resume_and_get(dev);
>   if (ret < 0) {
>   dev_err(dev, "Failed to get runtime sync for PCIe dev: %d\n",
>   ret);
> 

There are two places in the driver where pm_runtime_get_sync() is called.

Thanks
Jon

-- 
nvpublic

Re: [PATCH] dmaengine: tegra20: Fix runtime PM imbalance in tegra_dma_issue_pending

[Jon Hunter]

On 08/04/2021 08:11, Dinghao Liu wrote:
> pm_runtime_get_sync() will increase the rumtime PM counter
> even it returns an error. Thus a pairing decrement is needed
> to prevent refcount leak. Fix this by replacing this API with
> pm_runtime_resume_and_get(), which will not change the runtime
> PM counter on error.
> 
> Signed-off-by: Dinghao Liu 
> ---
>  drivers/dma/tegra20-apb-dma.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
> index 71827d9b0aa1..73178afaf4c2 100644
> --- a/drivers/dma/tegra20-apb-dma.c
> +++ b/drivers/dma/tegra20-apb-dma.c
> @@ -723,7 +723,7 @@ static void tegra_dma_issue_pending(struct dma_chan *dc)
>   goto end;
>   }
>   if (!tdc->busy) {
> - err = pm_runtime_get_sync(tdc->tdma->dev);
> + err = pm_runtime_resume_and_get(tdc->tdma->dev);
>   if (err < 0) {
>   dev_err(tdc2dev(tdc), "Failed to enable DMA\n");
>   goto end;
> 


Thanks! Looks like there are two instances of this that need fixing.

Cheers
Jon

-- 
nvpublic

Re: Regression v5.12-rc3: net: stmmac: re-init rx buffers when mac resume back

[Jon Hunter]

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. 

Jon

-- 
nvpublic

Re: Regression v5.12-rc3: net: stmmac: re-init rx buffers when mac resume back

[Jon Hunter]

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.

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.

Thanks
Jon

-- 
nvpublic

Re: Regression v5.12-rc3: net: stmmac: re-init rx buffers when mac resume back

[Jon Hunter]

On 31/03/2021 08:43, 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?


I am not sure either and I don't see anything obvious.

Guiseppe, Alexandre, Jose, do you see anything that is wrong with
Joakim's change 9c63faaa931e? This is completely breaking resume from
suspend on one of our boards and I would like to get your inputs?

Thanks
Jon

-- 
nvpublic

Re: Regression v5.12-rc3: net: stmmac: re-init rx buffers when mac resume back

[Jon Hunter]

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?

Thanks
Jon

-- 
nvpublic

Re: Regression v5.12-rc3: net: stmmac: re-init rx buffers when mac resume back

[Jon Hunter]

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?

>>> 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?

Jon

-- 
nvpublic

Re: Regression v5.12-rc3: net: stmmac: re-init rx buffers when mac resume back

[Jon Hunter]

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.

> 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?

Jon

-- 
nvpublic

Regression v5.12-rc3: net: stmmac: re-init rx buffers when mac resume back

[Jon Hunter]

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.

Thanks
Jon

Re: [PATCH v1 2/3] driver core: Update device link status properly for device_bind_driver()

[Jon Hunter]

On 02/03/2021 21:11, Saravana Kannan wrote:
> Device link status was not getting updated correctly when
> device_bind_driver() is called on a device. This causes a warning[1].
> Fix this by updating device links that can be updated and dropping
> device links that can't be updated to a sensible state.
> 
> [1] - 
> https://lore.kernel.org/lkml/56f7d032-ba5a-a8c7-23de-2969d98c5...@nvidia.com/
> Signed-off-by: Saravana Kannan 
> ---
>  drivers/base/base.h |  1 +
>  drivers/base/core.c | 35 +++
>  drivers/base/dd.c   |  4 +++-
>  3 files changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/base/base.h b/drivers/base/base.h
> index 52b3d7b75c27..1b44ed588f66 100644
> --- a/drivers/base/base.h
> +++ b/drivers/base/base.h
> @@ -185,6 +185,7 @@ extern int device_links_read_lock(void);
>  extern void device_links_read_unlock(int idx);
>  extern int device_links_read_lock_held(void);
>  extern int device_links_check_suppliers(struct device *dev);
> +extern void device_links_force_bind(struct device *dev);
>  extern void device_links_driver_bound(struct device *dev);
>  extern void device_links_driver_cleanup(struct device *dev);
>  extern void device_links_no_driver(struct device *dev);
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index f29839382f81..45c75cc96fdc 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -1153,6 +1153,41 @@ static ssize_t waiting_for_supplier_show(struct device 
> *dev,
>  }
>  static DEVICE_ATTR_RO(waiting_for_supplier);
>  
> +/**
> + * device_links_force_bind - Prepares device to be force bound
> + * @dev: Consumer device.
> + *
> + * device_bind_driver() force binds a device to a driver without calling any
> + * driver probe functions. So the consumer really isn't going to wait for any
> + * supplier before it's bound to the driver. We still want the device link
> + * states to be sensible when this happens.
> + *
> + * In preparation for device_bind_driver(), this function goes through each
> + * supplier device links and checks if the supplier is bound. If it is, then
> + * the device link status is set to CONSUMER_PROBE. Otherwise, the device 
> link
> + * is dropped. Links without the DL_FLAG_MANAGED flag set are ignored.
> + */
> +void device_links_force_bind(struct device *dev)
> +{
> + struct device_link *link, *ln;
> +
> + device_links_write_lock();
> +
> + list_for_each_entry_safe(link, ln, >links.suppliers, c_node) {
> + if (!(link->flags & DL_FLAG_MANAGED))
> + continue;
> +
> + if (link->status != DL_STATE_AVAILABLE) {
> + device_link_drop_managed(link);
> + continue;
> + }
> + WRITE_ONCE(link->status, DL_STATE_CONSUMER_PROBE);
> + }
> + dev->links.status = DL_DEV_PROBING;
> +
> + device_links_write_unlock();
> +}
> +
>  /**
>   * device_links_driver_bound - Update device links after probing its driver.
>   * @dev: Device to update the links for.
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index f18963f42e21..eb201c6d5a6a 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -460,8 +460,10 @@ int device_bind_driver(struct device *dev)
>   int ret;
>  
>   ret = driver_sysfs_add(dev);
> - if (!ret)
> + if (!ret) {
> + device_links_force_bind(dev);
>       driver_bound(dev);
> + }
>   else if (dev->bus)
>   blocking_notifier_call_chain(>bus->p->bus_notifier,
>BUS_NOTIFY_DRIVER_NOT_BOUND, dev);
> 

Thanks, this fixes the problem I had observed.

Tested-by: Jon Hunter 

Cheers!
Jon

-- 
nvpublic

Re: [PATCH] ASoC: soc-core: fix DMI handling

[Jon Hunter]

On 10/03/2021 19:39, Pierre-Louis Bossart wrote:
> When DMI information is not present, trying to assign the card long
> name results in the following warning.
> 
> WARNING KERN tegra-audio-graph-card sound: ASoC: no DMI vendor name!
> 
> The initial solution suggested was to test if the card device is an
> ACPI one. This causes a regression visible to userspace on all Intel
> platforms, with UCM unable to load card profiles based on DMI
> information: the card devices are not necessarily ACPI ones, e.g. when
> the parent creates platform devices on Intel devices.
> 
> To fix this problem, this patch exports the existing dmi_available
> variable and tests it in the ASoC core.
> 
> Fixes: c014170408bc ("ASoC: soc-core: Prevent warning if no DMI table is 
> present")
> Signed-off-by: Pierre-Louis Bossart 
> ---
>  drivers/firmware/dmi_scan.c | 1 +
>  sound/soc/soc-core.c| 2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
> index d51ca0428bb8..f191a1f901ac 100644
> --- a/drivers/firmware/dmi_scan.c
> +++ b/drivers/firmware/dmi_scan.c
> @@ -166,6 +166,7 @@ static int __init dmi_checksum(const u8 *buf, u8 len)
>  static const char *dmi_ident[DMI_STRING_MAX];
>  static LIST_HEAD(dmi_devices);
>  int dmi_available;
> +EXPORT_SYMBOL_GPL(dmi_available);
>  
>  /*
>   *   Save a DMI string
> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> index 16ba54eb8164..c7e4600b2dd4 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -1574,7 +1574,7 @@ int snd_soc_set_dmi_name(struct snd_soc_card *card, 
> const char *flavour)
>   if (card->long_name)
>   return 0; /* long name already set by driver or from DMI */
>  
> - if (!is_acpi_device_node(card->dev->fwnode))
> + if (!dmi_available)
>           return 0;
>  
>   /* make up dmi long name as: vendor-product-version-board */
> 


Thanks for fixing.

Reviewed-by: Jon Hunter 
Tested-by: Jon Hunter 

Cheers
Jon

-- 
nvpublic

Re: [PATCH V2] ASoC: soc-core: Prevent warning if no DMI table is present

[Jon Hunter]

On 10/03/2021 18:37, Pierre-Louis Bossart wrote:
> 
 Build time dependencies aren't going to help anything, arm64 (and to my
 understanding some future x86 systems, LynxPoint IIRC) supports both DT
 and ACPI and so you have kernels built with support for both.
>>
>>> well, that's what I suggested initially:
>>>     if (is_of_node(card->dev->fwnode))
>>
>>> I used the of_node test as a proxy for 'no DMI' since I am not aware
>>> of any
>>> means to detect if DMI is enabled at run-time.
>>
>> Can we not fix the DMI code so it lets us check dmi_available either
>> directly or with an accessor?  I don't understand why all the proposals
>> are dancing around local bodges here.
> 
> something like this then (compile-tested only)?
> 
> diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
> index d51ca0428bb8..f191a1f901ac 100644
> --- a/drivers/firmware/dmi_scan.c
> +++ b/drivers/firmware/dmi_scan.c
> @@ -166,6 +166,7 @@ static int __init dmi_checksum(const u8 *buf, u8 len)
>  static const char *dmi_ident[DMI_STRING_MAX];
>  static LIST_HEAD(dmi_devices);
>  int dmi_available;
> +EXPORT_SYMBOL_GPL(dmi_available);
> 
>  /*
>   * Save a DMI string
> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> index 16ba54eb8164..c7e4600b2dd4 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -1574,7 +1574,7 @@ int snd_soc_set_dmi_name(struct snd_soc_card
> *card, const char *flavour)
>     if (card->long_name)
>     return 0; /* long name already set by driver or from DMI */
> 
> -   if (!is_acpi_device_node(card->dev->fwnode))
> +   if (!dmi_available)
>     return 0;
> 
>     /* make up dmi long name as: vendor-product-version-board */


Sounds good to me. I would have done the same if I had known that the
current solution would have caused this regression.

Cheers
Jon

-- 
nvpublic

[PATCH V2] ASoC: soc-core: Prevent warning if no DMI table is present

[Jon Hunter]

Many systems do not use ACPI and hence do not provide a DMI table. On
non-ACPI systems a warning, such as the following, is printed on boot.

 WARNING KERN tegra-audio-graph-card sound: ASoC: no DMI vendor name!

The variable 'dmi_available' is not exported and so currently cannot be
used by kernel modules without adding an accessor. However, it is
possible to use the function is_acpi_device_node() to determine if the
sound card is an ACPI device and hence indicate if we expect a DMI table
to be present. Therefore, call is_acpi_device_node() to see if we are
using ACPI and only parse the DMI table if we are booting with ACPI.

Signed-off-by: Jon Hunter 
---
Changes since V1:
- Use is_acpi_device_node() to determine if we expect the DMI table to
  be present.

 sound/soc/soc-core.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index f6d4e99b590c..0cffc9527e28 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1573,6 +1574,9 @@ int snd_soc_set_dmi_name(struct snd_soc_card *card, const 
char *flavour)
if (card->long_name)
return 0; /* long name already set by driver or from DMI */
 
+   if (!is_acpi_device_node(card->dev->fwnode))
+   return 0;
+
/* make up dmi long name as: vendor-product-version-board */
vendor = dmi_get_system_info(DMI_BOARD_VENDOR);
if (!vendor || !is_dmi_valid(vendor)) {
-- 
2.25.1

Re: [PATCH] ASoC: soc-core: Prevent warning if no DMI table is present

[Jon Hunter]

On 02/03/2021 17:23, Takashi Iwai wrote:
> On Tue, 02 Mar 2021 13:49:13 +0100,
> Mark Brown wrote:
>>
>> On Tue, Mar 02, 2021 at 09:27:12AM +, Jon Hunter wrote:
>>> Many systems do not provide a DMI table and on these systems a warning,
>>> such as the following, is printed on boot ...
>>>
>>>  WARNING KERN tegra-audio-graph-card sound: ASoC: no DMI vendor name!
>>>
>>> If DMI support is enabled in the kernel, there is no simple way to
>>> detect if a DMI table is table or not. Note that the variable
>>> 'dmi_available' is not exported and so cannot be used by kernel modules.
>>
>> We could fix that, or provide an accessor function?  Or only warn if
>> we're on an ACPI system (which we can check from a module).  This really
>> does feel like something we should be warning about on systems that are
>> supposed to have DMI information available as standard.
> 
> I had the same feeling when I took a look at the patch, but later
> changed mind, since the error will pop up also if a system has an
> unmatured BIOS with some bogus vendor DMI string, too.  That is, users
> of such a machine gets always an error message although this isn't any
> serious problem unless you need a dedicated UCM profile (which can't
> be used on such a system in anyway).
> 
> So, I agree that exporting dmi_avilable could improve the situation a
> bit, but the error level needs still reconsideration.


Thanks for the feedback. Yes it is probably better if we have a specific
test. I will send out a V2 for review.

Thanks
Jon

-- 
nvpublic

[PATCH] ASoC: soc-core: Prevent warning if no DMI table is present

[Jon Hunter]

Many systems do not provide a DMI table and on these systems a warning,
such as the following, is printed on boot ...

 WARNING KERN tegra-audio-graph-card sound: ASoC: no DMI vendor name!

If DMI support is enabled in the kernel, there is no simple way to
detect if a DMI table is table or not. Note that the variable
'dmi_available' is not exported and so cannot be used by kernel modules.
It could be possible to have every ASoC sound card driver set the long
name to avoid the above message, but it might be intentional for the
long name, that we fall back to using the sound card name. Therefore,
make this a debug print by default to avoid the warning.

Signed-off-by: Jon Hunter 
---
 sound/soc/soc-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index f6d4e99b590c..f1189e7c1fcc 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1576,7 +1576,7 @@ int snd_soc_set_dmi_name(struct snd_soc_card *card, const 
char *flavour)
/* make up dmi long name as: vendor-product-version-board */
vendor = dmi_get_system_info(DMI_BOARD_VENDOR);
if (!vendor || !is_dmi_valid(vendor)) {
-   dev_warn(card->dev, "ASoC: no DMI vendor name!\n");
+   dev_dbg(card->dev, "ASoC: no DMI vendor name!\n");
return 0;
}
 
-- 
2.25.1

Re: phy_attach_direct()'s use of device_bind_driver()

[Jon Hunter]

On 10/02/2021 22:56, Andrew Lunn wrote:
> On Wed, Feb 10, 2021 at 02:13:48PM -0800, Saravana Kannan wrote:
>> Hi,
>>
>> This email was triggered by this other email[1].
> 
> And it appears the Tegra194 Jetson Xavier uses the Marvell 88E1512
> PHY. So ensure the Marvell driver is available, and it should get
> probed in the usual way, the fallback driver will not be needed.


Yes that is correct. Enabling the Marvell PHY does fix this indeed and
so I can enable that as part of our testsuite. We were seeing the same
warning on Tegra186 Jetson TX2 and enabling the BRCM PHY resolves that
as well. I will ensure that these are enabled going forward.

Cheers
Jon

-- 
nvpublic

Re: [PATCH v1 2/2] ASoC: tegra: Add RT5631 machine driver

[Jon Hunter]

On 02/02/2021 15:25, Dmitry Osipenko wrote:
> 02.02.2021 16:22, Jon Hunter пишет:
>> So this is very similar to tegra_rt5677, is it not possible to support
>> both codecs with the same machine driver?
> 
> These codecs require individual configurations and those
> "../codecs/rt5631.h" and  "../codecs/rt5677.h" aren't compatible at a
> quick glance.

Right but not all of that is needed. What is actually needed from the
header files?

> The tegra_rt5677 also uses outdated GPIO API and etc. Hence the new
> driver should be a better base anyways.

Sounds like a good time to update it :-)

> Overall it shouldn't worth time and effort trying to squeeze these
> drivers into a single one, IMO.

Not sure I agree when these drivers appear to be 90% the same.

Jon

-- 
nvpublic

Re: [PATCH v1 0/2] ASoC: tegra: Add RT5631 machine driver

[Jon Hunter]

On 31/01/2021 18:40, Ion Agorria wrote:
> From: Ion Agorria 
> 
> Adds support for Tegra SoC devices with RT5631 sound codec.
> Playback to speakers, headphones and internal mic recording works.
> 
> This driver is used for ASUS Transformer TF201, TF700T and others
> Tegra based devices containing RT5631.
> 
> Svyatoslav Ryhel (2):
>   ASoC: dt-bindings: tegra: Add binding for RT5631
>   ASoC: tegra: Add RT5631 machine driver
> 
>  .../sound/nvidia,tegra-audio-rt5631.yaml  | 134 +
>  sound/soc/tegra/Kconfig   |   8 +
>  sound/soc/tegra/Makefile  |   2 +
>  sound/soc/tegra/tegra_rt5631.c| 261 ++
>  4 files changed, 405 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/sound/nvidia,tegra-audio-rt5631.yaml
>  create mode 100644 sound/soc/tegra/tegra_rt5631.c


I don't see any user of this driver included. Any reason why?

Jon

-- 
nvpublic

Re: [PATCH v1 2/2] ASoC: tegra: Add RT5631 machine driver

[Jon Hunter]

On 31/01/2021 18:41, Ion Agorria wrote:
> From: Svyatoslav Ryhel 
> 
> Add Tegra ASoC driver for Realtek ALC5631/RT5631 codec. The RT5631
> codec is found on devices like ASUS Transformer TF201, TF700T and other
> Tegra-based Android tablets.
> 
> Signed-off-by: Svyatoslav Ryhel 
> Signed-off-by: Ion Agorria 
> ---
>  sound/soc/tegra/Kconfig|   8 +
>  sound/soc/tegra/Makefile   |   2 +
>  sound/soc/tegra/tegra_rt5631.c | 261 +
>  3 files changed, 271 insertions(+)
>  create mode 100644 sound/soc/tegra/tegra_rt5631.c
> 
> diff --git a/sound/soc/tegra/Kconfig b/sound/soc/tegra/Kconfig
> index acaf7339168d..449a858f155d 100644
> --- a/sound/soc/tegra/Kconfig
> +++ b/sound/soc/tegra/Kconfig
> @@ -126,6 +126,14 @@ config SND_SOC_TEGRA_AUDIO_GRAPH_CARD
> few things for Tegra audio. Most of the code is re-used from
> audio graph driver and the same DT bindings are used.
>  
> +config SND_SOC_TEGRA_RT5631
> + tristate "SoC Audio support for Tegra boards using an RT5631 codec"
> + depends on SND_SOC_TEGRA && I2C && GPIOLIB
> + select SND_SOC_RT5631
> + help
> +   Say Y or M here if you want to add support for SoC audio on Tegra
> +   boards using the RT5631 codec, such as Transformer.
> +
>  config SND_SOC_TEGRA_RT5640
>   tristate "SoC Audio support for Tegra boards using an RT5640 codec"
>   depends on SND_SOC_TEGRA && I2C && GPIOLIB
> diff --git a/sound/soc/tegra/Makefile b/sound/soc/tegra/Makefile
> index af0b9889306c..11debfc03bc4 100644
> --- a/sound/soc/tegra/Makefile
> +++ b/sound/soc/tegra/Makefile
> @@ -29,6 +29,7 @@ obj-$(CONFIG_SND_SOC_TEGRA186_DSPK) += 
> snd-soc-tegra186-dspk.o
>  obj-$(CONFIG_SND_SOC_TEGRA210_ADMAIF) += snd-soc-tegra210-admaif.o
>  
>  # Tegra machine Support
> +snd-soc-tegra-rt5631-objs := tegra_rt5631.o
>  snd-soc-tegra-rt5640-objs := tegra_rt5640.o
>  snd-soc-tegra-rt5677-objs := tegra_rt5677.o
>  snd-soc-tegra-wm8753-objs := tegra_wm8753.o
> @@ -41,6 +42,7 @@ snd-soc-tegra-max98090-objs := tegra_max98090.o
>  snd-soc-tegra-sgtl5000-objs := tegra_sgtl5000.o
>  snd-soc-tegra-audio-graph-card-objs := tegra_audio_graph_card.o
>  
> +obj-$(CONFIG_SND_SOC_TEGRA_RT5631) += snd-soc-tegra-rt5631.o
>  obj-$(CONFIG_SND_SOC_TEGRA_RT5640) += snd-soc-tegra-rt5640.o
>  obj-$(CONFIG_SND_SOC_TEGRA_RT5677) += snd-soc-tegra-rt5677.o
>  obj-$(CONFIG_SND_SOC_TEGRA_WM8753) += snd-soc-tegra-wm8753.o
> diff --git a/sound/soc/tegra/tegra_rt5631.c b/sound/soc/tegra/tegra_rt5631.c
> new file mode 100644
> index ..9034f48bcb26
> --- /dev/null
> +++ b/sound/soc/tegra/tegra_rt5631.c
> @@ -0,0 +1,261 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * tegra_rt5631.c - Tegra machine ASoC driver for boards using RT5631 codec.
> + *
> + * Copyright (c) 2020, Svyatoslav Ryhel and Ion Agorria

2021 now :-)

> + *
> + * Based on code copyright/by:
> + *
> + * Copyright (c) 2012, NVIDIA CORPORATION.  All rights reserved.
> + * Author: Stephen Warren 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "tegra_asoc_utils.h"
> +
> +#include "../codecs/rt5631.h"
> +
> +struct tegra_rt5631 {
> + struct tegra_asoc_utils_data util_data;
> + struct gpio_desc *gpiod_hp_mute;
> + struct gpio_desc *gpiod_hp_det;
> +};
> +
> +static int tegra_rt5631_hw_params(struct snd_pcm_substream *substream,
> +   struct snd_pcm_hw_params *params)
> +{
> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
> + struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
> + struct snd_soc_card *card = rtd->card;
> + struct tegra_rt5631 *machine = snd_soc_card_get_drvdata(card);
> + unsigned int srate, mclk;
> + int err;
> +
> + srate = params_rate(params);
> + switch (srate) {
> + case 64000:
> + case 88200:
> + case 96000:
> + mclk = 128 * srate;
> + break;
> + default:
> + mclk = 256 * srate;
> + break;
> + }
> + /* FIXME: Codec only requires >= 3MHz if OSR==0 */
> + while (mclk < 600)
> + mclk *= 2;

I don't understand that above. You say greater or equal to 3MHz, but
then compare against 6MHz. Please explain and elborate a bit more in the
comment.

> +
> + err = tegra_asoc_utils_set_rate(>util_data, srate, mclk);
> + if (err < 0) {
> + dev_err(card->dev, "Can't configure clocks\n");
> + return err;
> + }
> +
> + err = snd_soc_dai_set_sysclk(codec_dai, 0, mclk, SND_SOC_CLOCK_IN);
> + if (err < 0) {
> + dev_err(card->dev, "codec_dai clock not set\n");
> + return err;
> + }
> +
> + return 0;
> +}
> +
> +static struct snd_soc_ops tegra_rt5631_ops = {
> + .hw_params = tegra_rt5631_hw_params,
> +};
> +
> +static struct snd_soc_jack tegra_rt5631_hp_jack;
> +
> +static 

Re: [PATCH v1 1/2] ASoC: dt-bindings: tegra: Add binding for RT5631

[Jon Hunter]

On 31/01/2021 18:41, Ion Agorria wrote:
> From: Svyatoslav Ryhel 
> 
> Add device-tree binding that describes hardware integration of RT5631
> audio codec chip with NVIDIA Tegra SoCs.
> 
> Signed-off-by: Svyatoslav Ryhel 
> Signed-off-by: Ion Agorria 
> ---
>  .../sound/nvidia,tegra-audio-rt5631.yaml  | 134 ++
>  1 file changed, 134 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/sound/nvidia,tegra-audio-rt5631.yaml
> 
> diff --git 
> a/Documentation/devicetree/bindings/sound/nvidia,tegra-audio-rt5631.yaml 
> b/Documentation/devicetree/bindings/sound/nvidia,tegra-audio-rt5631.yaml
> new file mode 100644
> index ..6ee62c599518
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/nvidia,tegra-audio-rt5631.yaml
> @@ -0,0 +1,134 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +
> +$id: http://devicetree.org/schemas/sound/nvidia,tegra-audio-rt5631.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NVIDIA Tegra RT5631 ASoC
> +
> +description: |
> +  This binding describes integration of the Realtek ALC5631/RT5631 sound
> +  codec with the sound system of NVIDIA Tegra SoCs.
> +
> +maintainers:
> +  - Jon Hunter 
> +  - Stephen Warren 
> +  - Thierry Reding 


Thierry and I should be sufficient and so no need to include Stephen in
the list.

> +
> +properties:
> +  compatible:
> +enum:
> +  - nvidia,tegra-audio-rt5631
> +
> +  clocks:
> +minItems: 3
> +items:
> +  - description: PLL A clock
> +  - description: PLL A OUT0 clock
> +  - description: The Tegra cdev1/extern1 clock, which feeds the card's 
> mclk
> +
> +  clock-names:
> +minItems: 3
> +items:
> +  - const: pll_a
> +  - const: pll_a_out0
> +  - const: mclk
> +
> +  assigned-clocks:
> +minItems: 1
> +maxItems: 3
> +
> +  assigned-clock-parents:
> +minItems: 1
> +maxItems: 3
> +
> +  assigned-clock-rates:
> +minItems: 1
> +maxItems: 3
> +
> +  nvidia,model:
> +$ref: /schemas/types.yaml#/definitions/string
> +description: User-visible name of this sound complex.
> +
> +  nvidia,audio-routing:
> +$ref: /schemas/types.yaml#/definitions/non-unique-string-array
> +description: |
> +  A list of the connections between audio components.
> +  Each entry is a pair of strings, the first being the connection's sink,
> +  the second being the connection's source. Valid names for sources and
> +  sinks are the RT5631's pins (as documented in its binding), and the 
> jacks
> +  on the board:
> +
> +  * Int Spk
> +  * Headphone Jack
> +  * Mic Jack
> +  * Int Mic
> +
> +  nvidia,i2s-controller:
> +$ref: /schemas/types.yaml#/definitions/phandle
> +description: Phandle of the Tegra I2S controller.
> +
> +  nvidia,audio-codec:
> +$ref: /schemas/types.yaml#/definitions/phandle
> +description: Phandle of the RT5631 audio codec.
> +
> +  nvidia,hp-mute-gpios:
> +description: GPIO that mutes the headphones (button event).
> +maxItems: 1
> +
> +  nvidia,hp-det-gpios:
> +description: GPIO that detects headphones plug-in.
> +maxItems: 1
> +
> +additionalProperties: false
> +
> +required:
> +  - compatible
> +  - clocks
> +  - clock-names
> +  - assigned-clocks
> +  - assigned-clock-parents
> +  - nvidia,model
> +  - nvidia,audio-routing
> +  - nvidia,i2s-controller
> +  - nvidia,audio-codec
> +
> +examples:
> +  - |
> +#include 
> +#include 
> +#include 
> +
> +sound {
> +compatible = "nvidia,tegra-audio-rt5631";
> +nvidia,model = "NVIDIA Tegra RT5631";
> +
> +nvidia,audio-routing =
> +"Headphone Jack", "HPOL",
> +"Headphone Jack", "HPOR",
> +"Int Spk", "SPOL",
> +"Int Spk", "SPOR",
> +"MIC1", "MIC Bias1",
> +"MIC Bias1", "Mic Jack",
> +"DMIC", "Int Mic";
> +
> +nvidia,i2s-controller = <_i2s1>;
> +nvidia,audio-codec = <>;
> +
> +nvidia,hp-det-gpios = < 178 GPIO_ACTIVE_LOW>;
> +nvidia,hp-mute-gpios = < 186 GPIO_ACTIVE_LOW>;
> +
> +clocks = <_car TEGRA30_CLK_PLL_A>,
> + <_car TEGRA30_CLK_PLL_A_OUT0>,
> + <_pmc TEGRA_PMC_CLK_OUT_1>;
> +clock-names = "pll_a", "pll_a_out0", "mclk";
> +
> +assigned-clocks = <_car TEGRA30_CLK_EXTERN1>,
> +  <_pmc TEGRA_PMC_CLK_OUT_1>;
> +
> +assigned-clock-parents = <_car TEGRA30_CLK_PLL_A_OUT0>,
> + <_car TEGRA30_CLK_EXTERN1>;
> +};
> +
> +...
> 

Otherwise looks good to me ...

Reviewed-by: Jon Hunter 

Cheers
Jon

-- 
nvpublic

Re: [PATCH 4.4 00/24] 4.4.254-rc1 review

[Jon Hunter]

On 29/01/2021 18:21, Guenter Roeck wrote:
> On Fri, Jan 29, 2021 at 12:06:35PM +0100, Greg Kroah-Hartman wrote:
>> This is the start of the stable review cycle for the 4.4.254 release.
>> There are 24 patches in this series, all will be posted as a response
>> to this one.  If anyone has any issues with these being applied, please
>> let me know.


Replying here because I don't have the original.

Test results for stable-v4.4:
6 builds:   6 pass, 0 fail
12 boots:   12 pass, 0 fail
28 tests:   28 pass, 0 fail

Linux version:  4.4.254-rc1-ga1b3543d6297
Boards tested:  tegra124-jetson-tk1, tegra20-ventana,
tegra30-cardhu-a04

Tested-by: Jon Hunter 

Jon

-- 
nvpublic

Re: [PATCH 5.4 00/18] 5.4.94-rc1 review

[Jon Hunter]

On 29/01/2021 18:22, Guenter Roeck wrote:
> On Fri, Jan 29, 2021 at 12:07:14PM +0100, Greg Kroah-Hartman wrote:
>> This is the start of the stable review cycle for the 5.4.94 release.
>> There are 18 patches in this series, all will be posted as a response
>> to this one.  If anyone has any issues with these being applied, please
>> let me know.

Replying here because I don't have the original.

Test results for stable-v5.4:
12 builds:  12 pass, 0 fail
26 boots:   26 pass, 0 fail
57 tests:   57 pass, 0 fail

Linux version:  5.4.94-rc1-g5a6e0182cbe9
Boards tested:  tegra124-jetson-tk1, tegra186-p2771-,
tegra194-p2972-, tegra20-ventana,
tegra210-p2371-2180, tegra210-p3450-,
tegra30-cardhu-a04

Tested-by: Jon Hunter 

Jon

-- 
nvpublic

Re: [PATCH 5.10 00/32] 5.10.12-rc1 review

[Jon Hunter]

On 29/01/2021 18:22, Naresh Kamboju wrote:
> On Fri, 29 Jan 2021 at 16:47, Greg Kroah-Hartman
>  wrote:
>>
>> This is the start of the stable review cycle for the 5.10.12 release.
>> There are 32 patches in this series, all will be posted as a response
>> to this one.  If anyone has any issues with these being applied, please
>> let me know.
Replying here because I don't have the original.

Test results for stable-v5.10:
12 builds:  12 pass, 0 fail
26 boots:   26 pass, 0 fail
65 tests:   65 pass, 0 fail

Linux version:  5.10.12-rc1-g324e71045b28
Boards tested:  tegra124-jetson-tk1, tegra186-p2771-,
tegra194-p2972-, tegra20-ventana,
tegra210-p2371-2180, tegra210-p3450-,
        tegra30-cardhu-a04

Tested-by: Jon Hunter 

Jon


-- 
nvpublic

Re: [PATCH 4.19 00/26] 4.19.172-rc1 review

[Jon Hunter]

On 29/01/2021 18:22, Guenter Roeck wrote:
> On Fri, Jan 29, 2021 at 12:06:57PM +0100, Greg Kroah-Hartman wrote:
>> This is the start of the stable review cycle for the 4.19.172 release.
>> There are 26 patches in this series, all will be posted as a response
>> to this one.  If anyone has any issues with these being applied, please
>> let me know.

Replying here because I don't have the original.

Test results for stable-v4.19:
12 builds:  12 pass, 0 fail
22 boots:   22 pass, 0 fail
38 tests:   38 pass, 0 fail

Linux version:  4.19.172-rc1-gd36f1541af5a
Boards tested:  tegra124-jetson-tk1, tegra186-p2771-,
tegra194-p2972-, tegra20-ventana,
tegra210-p2371-2180, tegra30-cardhu-a04

Tested-by: Jon Hunter 

Jon

-- 
nvpublic

Re: [PATCH 4.14 00/50] 4.14.218-rc1 review

[Jon Hunter]

On 29/01/2021 18:21, Guenter Roeck wrote:
> On Fri, Jan 29, 2021 at 12:06:25PM +0100, Greg Kroah-Hartman wrote:
>> This is the start of the stable review cycle for the 4.14.218 release.
>> There are 50 patches in this series, all will be posted as a response
>> to this one.  If anyone has any issues with these being applied, please
>> let me know.


Replying here because I don't have the original.

Test results for stable-v4.14:
8 builds:   8 pass, 0 fail
16 boots:   16 pass, 0 fail
30 tests:   30 pass, 0 fail

Linux version:  4.14.218-rc1-g672a9e33037f
Boards tested:  tegra124-jetson-tk1, tegra20-ventana,
tegra210-p2371-2180, tegra30-cardhu-a04

Tested-by: Jon Hunter 

Jon

-- 
nvpublic

Re: [PATCH v1 0/5] Enable fw_devlink=on by default

[Jon Hunter]

On 14/01/2021 16:56, Jon Hunter wrote:
> 
> On 14/01/2021 16:47, Saravana Kannan wrote:
> 
> ...
> 
>>> Yes this is the warning shown here [0] and this is coming from
>>> the 'Generic PHY stmmac-0:00' device.
>>
>> Can you print the supplier and consumer device when this warning is
>> happening and let me know? That'd help too. I'm guessing the phy is
>> the consumer.
> 
> 
> Sorry I should have included that. I added a print to dump this on
> another build but failed to include here.
> 
> WARNING KERN Generic PHY stmmac-0:00: supplier 220.gpio (status 1)
> 
> The status is the link->status and looks like the supplier is the
> gpio controller. I have verified that the gpio controller is probed
> before this successfully.
> 
>> So the warning itself isn't a problem -- it's not breaking anything or
>> leaking memory or anything like that. But the device link is jumping
>> states in an incorrect manner. With enough context of this code (why
>> the device_bind_driver() is being called directly instead of going
>> through the normal probe path), it should be easy to fix (I'll just
>> need to fix up the device link state).
> 
> Correct, the board seems to boot fine, we just get this warning.


Have you had chance to look at this further?

The following does appear to avoid the warning, but I am not sure if
this is the correct thing to do ...

index 9179825ff646..095aba84f7c2 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -456,6 +456,10 @@ int device_bind_driver(struct device *dev)
 {
int ret;

+   ret = device_links_check_suppliers(dev);
+   if (ret)
+   return ret;
+
ret = driver_sysfs_add(dev);
if (!ret)
driver_bound(dev);


Cheers
Jon

-- 
nvpublic

Re: [PATCH] arm64: tegra: Enable Jetson-Xavier J512 USB host

[Jon Hunter]

On 19/01/2021 02:23, JC Kuo wrote:
> This commit enables USB host mode at J512 type-C port of Jetson-Xavier.
> 
> Signed-off-by: JC Kuo 
> ---
>  .../arm64/boot/dts/nvidia/tegra194-p2888.dtsi |  8 +++
>  .../boot/dts/nvidia/tegra194-p2972-.dts   | 24 +--
>  2 files changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi 
> b/arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi
> index d71b7a1140fe..7e7b0eb90c80 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra194-p2888.dtsi
> @@ -93,6 +93,10 @@ padctl@352 {
>   vclamp-usb-supply = <_1v8ao>;
>  
>   ports {
> + usb2-0 {
> + vbus-supply = <_5v0_sys>;
> + };
> +
>   usb2-1 {
>   vbus-supply = <_5v0_sys>;
>   };
> @@ -105,6 +109,10 @@ usb3-0 {
>   vbus-supply = <_5v0_sys>;
>   };
>  
> + usb3-2 {
> + vbus-supply = <_5v0_sys>;
> + };
> +
>   usb3-3 {
>   vbus-supply = <_5v0_sys>;
>   };
> diff --git a/arch/arm64/boot/dts/nvidia/tegra194-p2972-.dts 
> b/arch/arm64/boot/dts/nvidia/tegra194-p2972-.dts
> index 54d057beec59..8697927b1fe7 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra194-p2972-.dts
> +++ b/arch/arm64/boot/dts/nvidia/tegra194-p2972-.dts
> @@ -57,6 +57,10 @@ padctl@352 {
>   pads {
>   usb2 {
>   lanes {
> + usb2-0 {
> + status = "okay";
> + };
> +
>   usb2-1 {
>   status = "okay";
>   };
> @@ -73,6 +77,10 @@ usb3-0 {
>   status = "okay";
>   };
>  
> + usb3-2 {
> + status = "okay";
> + };
> +
>   usb3-3 {
>   status = "okay";
>   };
> @@ -81,6 +89,11 @@ usb3-3 {
>   };
>  
>   ports {
> + usb2-0 {
> + mode = "host";
> + status = "okay";
> + };
> +
>   usb2-1 {
>   mode = "host";
>   status = "okay";
> @@ -96,6 +109,11 @@ usb3-0 {
>   status = "okay";
>   };
>  
> + usb3-2 {
> + nvidia,usb2-companion = <0>;
> + status = "okay";
> + };
> +
>   usb3-3 {
>   nvidia,usb2-companion = <3>;
>   maximum-speed = "super-speed";
> @@ -107,11 +125,13 @@ usb3-3 {
>   usb@361 {
>   status = "okay";
>  
> - phys =  
> <&{/bus@0/padctl@352/pads/usb2/lanes/usb2-1}>,
> + phys =  
> <&{/bus@0/padctl@352/pads/usb2/lanes/usb2-0}>,
> + 
> <&{/bus@0/padctl@352/pads/usb2/lanes/usb2-1}>,
>   
> <&{/bus@0/padctl@352/pads/usb2/lanes/usb2-3}>,
>           
> <&{/bus@0/padctl@352/pads/usb3/lanes/usb3-0}>,
> + 
> <&{/bus@0/padctl@352/pads/usb3/lanes/usb3-2}>,
>   
> <&{/bus@0/padctl@352/pads/usb3/lanes/usb3-3}>;
> - phy-names = "usb2-1", "usb2-3", "usb3-0", "usb3-3";
> + phy-names = "usb2-0", "usb2-1", "usb2-3", "usb3-0", 
> "usb3-2", "usb3-3";
>   };
>  
>   pwm@c34 {
> 

Thanks. Works for me.

Acked-by: Jon Hunter 
Tested-by: Jon Hunter 

Cheers
Jon

-- 
nvpublic

Re: [PATCH] ACPICA: fix -Wfallthrough

[Jon Hunter]

On 11/11/2020 02:11, Nick Desaulniers wrote:
> The "fallthrough" pseudo-keyword was added as a portable way to denote
> intentional fallthrough. This code seemed to be using a mix of
> fallthrough comments that GCC recognizes, and some kind of lint marker.
> I'm guessing that linter hasn't been run in a while from the mixed use
> of the marker vs comments.
> 
> Signed-off-by: Nick Desaulniers 


I know this is not the exact version that was merged, I can't find it on
the list, but looks like the version that was merged [0], is causing
build errors with older toolchains (GCC v6) ...

/dvs/git/dirty/git-master_l4t-upstream/kernel/drivers/acpi/acpica/dscontrol.c: 
In function ‘acpi_ds_exec_begin_control_op’:
/dvs/git/dirty/git-master_l4t-upstream/kernel/drivers/acpi/acpica/dscontrol.c:65:3:
 error: ‘ACPI_FALLTHROUGH’ undeclared (first use in this function)
   ACPI_FALLTHROUGH;
   ^~~~
/dvs/git/dirty/git-master_l4t-upstream/kernel/drivers/acpi/acpica/dscontrol.c:65:3:
 note: each undeclared identifier is reported only once for each function it 
appears in
/dvs/git/dirty/git-master_l4t-upstream/kernel/scripts/Makefile.build:287: 
recipe for target 'drivers/acpi/acpica/dscontrol.o' failed

Cheers
Jon

[0] https://github.com/acpica/acpica/commit/4b9135f5
 
-- 
nvpublic

Re: [PATCH v1 0/5] Enable fw_devlink=on by default

[Jon Hunter]

On 14/01/2021 21:50, Saravana Kannan wrote:
> On Thu, Jan 14, 2021 at 10:55 AM Jon Hunter  wrote:
>>
>>
>> On 14/01/2021 16:52, Saravana Kannan wrote:
>>
>> ...
>>
>>> Thanks! I think you forgot to enable those logs though. Also, while
>>> you are at it, maybe enable the logs in device_link_add() too please?
>>
>>
>> Sorry try this one.
>>
>> Cheers
>> Jon
> 
> Phew! That took almost 4 hours to debug on the side! I think I figured
> it out. Can you try this patch? If it works or improves things, I'll
> explain why it helps.
> 
> -Saravana
> 
> diff --git a/drivers/of/property.c b/drivers/of/property.c
> index 5f9eed79a8aa..1c8c65c4a887 100644
> --- a/drivers/of/property.c
> +++ b/drivers/of/property.c
> @@ -1258,6 +1258,8 @@ DEFINE_SIMPLE_PROP(pinctrl5, "pinctrl-5", NULL)
>  DEFINE_SIMPLE_PROP(pinctrl6, "pinctrl-6", NULL)
>  DEFINE_SIMPLE_PROP(pinctrl7, "pinctrl-7", NULL)
>  DEFINE_SIMPLE_PROP(pinctrl8, "pinctrl-8", NULL)
> +DEFINE_SIMPLE_PROP(gpio_compat, "gpio", "#gpio-cells")
> +DEFINE_SIMPLE_PROP(gpios_compat, "gpios", "#gpio-cells")
>  DEFINE_SUFFIX_PROP(regulators, "-supply", NULL)
>  DEFINE_SUFFIX_PROP(gpio, "-gpio", "#gpio-cells")
>  DEFINE_SUFFIX_PROP(gpios, "-gpios", "#gpio-cells")
> @@ -1296,6 +1298,8 @@ static const struct supplier_bindings
> of_supplier_bindings[] = {
> { .parse_prop = parse_pinctrl6, },
> { .parse_prop = parse_pinctrl7, },
> { .parse_prop = parse_pinctrl8, },
> +   { .parse_prop = parse_gpio_compat, },
> +   { .parse_prop = parse_gpios_compat, },
> { .parse_prop = parse_regulators, },
> { .parse_prop = parse_gpio, },
> { .parse_prop = parse_gpios, },
> 

Thanks, that worked!

Tested-by: Jon Hunter 

Thanks for digging into that one. Would have taken me more than 4 hours!

Jon

-- 
nvpublic

Re: [Patch v2 0/4] Add Nvidia Tegra GPC-DMA driver

[Jon Hunter]

On 15/01/2021 05:56, Vinod Koul wrote:
> On 14-01-21, 10:11, Jon Hunter wrote:
>>
>> On 06/08/2020 08:30, Rajesh Gumasta wrote:
>>> Changes in patch v2:
>>> Addressed review comments in patch v1
>>
>>
>> Is there any update on this series? Would be good to get this upstream.
> 
> Not sure why, this is is not in my queue, can someone please resend this
> to me

Sorry, this question was meant for Rajesh. This series is not ready yet.
There are still some items that need to be addressed.

Thanks
Jon

-- 
nvpublic

Re: [PATCH v1 0/5] Enable fw_devlink=on by default

[Jon Hunter]

On 14/01/2021 16:47, Saravana Kannan wrote:

...

>> Yes this is the warning shown here [0] and this is coming from
>> the 'Generic PHY stmmac-0:00' device.
> 
> Can you print the supplier and consumer device when this warning is
> happening and let me know? That'd help too. I'm guessing the phy is
> the consumer.


Sorry I should have included that. I added a print to dump this on
another build but failed to include here.

WARNING KERN Generic PHY stmmac-0:00: supplier 220.gpio (status 1)

The status is the link->status and looks like the supplier is the
gpio controller. I have verified that the gpio controller is probed
before this successfully.

> So the warning itself isn't a problem -- it's not breaking anything or
> leaking memory or anything like that. But the device link is jumping
> states in an incorrect manner. With enough context of this code (why
> the device_bind_driver() is being called directly instead of going
> through the normal probe path), it should be easy to fix (I'll just
> need to fix up the device link state).


Correct, the board seems to boot fine, we just get this warning.

Cheers
Jon

-- 
nvpublic

Re: [PATCH v1 0/5] Enable fw_devlink=on by default

[Jon Hunter]

On 14/01/2021 16:40, Saravana Kannan wrote:
> On Thu, Jan 14, 2021 at 3:35 AM Jon Hunter  wrote:
>>
>>
>> On 13/01/2021 21:29, Saravana Kannan wrote:
>>
>> ...
>>
>>>> I am seeing the same problem on Tegra30 Cardhu A04 where several regulators
>>>> are continuously deferred and prevents the board from booting ...
>>>>
>>>> [2.518334] platform panel: probe deferral - supplier regulator@11 not 
>>>> ready
>>>>
>>>> [2.525503] platform regulator@1: probe deferral - supplier 4-002d not 
>>>> ready
>>>>
>>>> [2.533141] platform regulator@3: probe deferral - supplier 
>>>> regulator@101 not ready
>>>>
>>>> [2.540856] platform regulator@5: probe deferral - supplier 
>>>> regulator@101 not ready
>>>>
>>>> [2.548589] platform regulator@6: probe deferral - supplier 
>>>> regulator@101 not ready
>>>>
>>>> [2.556316] platform regulator@7: probe deferral - supplier 
>>>> regulator@101 not ready
>>>>
>>>> [2.564041] platform regulator@8: probe deferral - supplier 
>>>> regulator@101 not ready
>>>>
>>>> [2.571743] platform regulator@9: probe deferral - supplier 
>>>> regulator@101 not ready
>>>>
>>>> [2.579463] platform regulator@10: probe deferral - supplier 
>>>> regulator@101 not ready
>>>>
>>>> [2.587273] platform regulator@11: probe deferral - supplier 
>>>> regulator@101 not ready
>>>>
>>>> [2.595088] platform regulator@12: probe deferral - supplier 
>>>> regulator@104 not ready
>>>>
>>>> [2.603837] platform regulator@102: probe deferral - supplier 
>>>> regulator@104 not ready
>>>>
>>>> [2.611726] platform regulator@103: probe deferral - supplier 
>>>> regulator@104 not ready
>>>>
>>>> [2.620137] platform 3000.pcie: probe deferral - supplier regulator@5 
>>>> not ready
>>>
>>> Looks like this is not the whole log? Do you see any "wait for
>>> supplier" logs? That's what all these boot issues should boil down to.
>>> And as usual, pointer to DT for this board please.
>>
>> Ah yes I see ...
>>
>>  platform regulator@1: probe deferral - wait for supplier tps65911@2d
> 
> Do you mind sharing the full log please? It's hard to tell you
> anything useful with bits and pieces of logs.
> 
>> Yes the device-tree for this board can be found here [0]. Looks like
>> there is a circular dependency between the vddctrl_reg and vddcore_reg.
>> This is part of coupled regulators which have a two-way linkage [1]. So
>> this change appears to conflict with this.
> 
> fw_devlink doesn't track "regulator-coupled-with". So that's probably
> not it. Also, this patch series was made to handle simple cycles
> properly. It'll functionally disable the device links it created when
> it comes to probe ordering. Only two overlapping cycles might cause
> issues -- and even that, not all the time. So yeah, full log please.


No problem. Please find attached.

Cheers
Jon


-- 
nvpublic
[  111.269288] VFS: Unable to mount root fs via NFS.
[  111.274136] devtmpfs: mounted
[  111.278710] Freeing unused kernel memory: 1024K
[  111.297234] Run /sbin/init as init process
[  111.301355]   with arguments:
[  111.304328] /sbin/init
[  111.307076] netdevwait
[  111.309794]   with environment:
[  111.312940] HOME=/
[  111.315303] TERM=linux
[  111.318333] Run /etc/init as init process
[  111.322358]   with arguments:
[  111.325330] /etc/init
[  111.327984] netdevwait
[  111.330702]   with environment:
[  111.333847] HOME=/
[  111.336265] TERM=linux
[  111.339174] Run /bin/init as init process
[  111.343196]   with arguments:
[  111.346206] /bin/init
[  111.348837] netdevwait
[  111.351548]   with environment:
[  111.354692] HOME=/
[  111.357110] TERM=linux
[  111.360010] Run /bin/sh as init process
[  111.363858]   with arguments:
[  111.366860] /bin/sh
[  111.369316] netdevwait
[  111.372027]   with environment:
[  111.375170] HOME=/
[  111.377608] TERM=linux
[  111.380510] Kernel panic - not syncing: No working init found.  Try passing 
init= option to kernel. See Linux Documentation/admin-guide/init.rst for 
guidance.
[  111.394699] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 
5.11.0-rc3-next-20210112-gdf869cab4b35 #1
[  111.403415] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
[  111.409697] [] (unwind_backtrace) from [] 
(show_stack+0x10/

Re: [PATCH v1 0/5] Enable fw_devlink=on by default

[Jon Hunter]

On 13/01/2021 21:26, Saravana Kannan wrote:
> On Wed, Jan 13, 2021 at 3:30 AM Jon Hunter  wrote:
>>
>>
>> On 18/12/2020 03:16, Saravana Kannan wrote:
>>> As discussed in LPC 2020, cyclic dependencies in firmware that couldn't
>>> be broken using logic was one of the last remaining reasons
>>> fw_devlink=on couldn't be set by default.
>>>
>>> This series changes fw_devlink so that when a cyclic dependency is found
>>> in firmware, the links between those devices fallback to permissive mode
>>> behavior. This way, the rest of the system still benefits from
>>> fw_devlink, but the ambiguous cases fallback to permissive mode.
>>>
>>> Setting fw_devlink=on by default brings a bunch of benefits (currently,
>>> only for systems with device tree firmware):
>>> * Significantly cuts down deferred probes.
>>> * Device probe is effectively attempted in graph order.
>>> * Makes it much easier to load drivers as modules without having to
>>>   worry about functional dependencies between modules (depmod is still
>>>   needed for symbol dependencies).
>>
>>
>> One issue we have come across with this is the of_mdio.c driver. On
>> Tegra194 Jetson Xavier I am seeing the following ...
>>
>> boot: logs: [   4.194791] WARNING KERN WARNING: CPU: 0 PID: 1 at 
>> /dvs/git/dirty/git-master_l4t-upstream/kernel/drivers/base/core.c:1189 
>> device_links_driver_bound+0x240/0x260
>> boot: logs: [   4.207683] WARNING KERN Modules linked in:
>> boot: logs: [   4.210691] WARNING KERN CPU: 0 PID: 1 Comm: swapper/0 Not 
>> tainted 5.11.0-rc3-next-20210112-gdf869cab4b35 #1
>> boot: logs: [   4.219221] WARNING KERN Hardware name: NVIDIA Jetson AGX 
>> Xavier Developer Kit (DT)
>> boot: logs: [   4.225628] WARNING KERN pstate: 8049 (Nzcv daif +PAN 
>> -UAO -TCO BTYPE=--)
>> boot: logs: [   4.231542] WARNING KERN pc : 
>> device_links_driver_bound+0x240/0x260
>> boot: logs: [   4.236587] WARNING KERN lr : 
>> device_links_driver_bound+0xf8/0x260
>> boot: logs: [   4.241560] WARNING KERN sp : 800011f4b980
>> boot: logs: [   4.244819] WARNING KERN x29: 800011f4b980 x28: 
>> 8208a0a0
>> boot: logs: [   4.250051] WARNING KERN x27: 8208a080 x26: 
>> 
>> boot: logs: [   4.255271] WARNING KERN x25: 0003 x24: 
>> 800011b99000
>> boot: logs: [   4.260489] WARNING KERN x23: 0001 x22: 
>> 800011df14f0
>> boot: logs: [   4.265706] WARNING KERN x21: 800011f4b9f8 x20: 
>> 800011df1000
>> boot: logs: [   4.270934] WARNING KERN x19: 8208a000 x18: 
>> 0005
>> boot: logs: [   4.276166] WARNING KERN x17: 0007 x16: 
>> 0001
>> boot: logs: [   4.281382] WARNING KERN x15: 80030c90 x14: 
>> 805c9df8
>> boot: logs: [   4.286618] WARNING KERN x13:  x12: 
>> 80030c90
>> boot: logs: [   4.291847] WARNING KERN x11: 805c9da8 x10: 
>> 0040
>> boot: logs: [   4.297061] WARNING KERN x9 : 80030c98 x8 : 
>> 
>> boot: logs: [   4.302291] WARNING KERN x7 : 0009 x6 : 
>> 
>> boot: logs: [   4.307509] WARNING KERN x5 : 8010 x4 : 
>> 
>> boot: logs: [   4.312739] WARNING KERN x3 : 800011df1e38 x2 : 
>> 80908c10
>> boot: logs: [   4.317956] WARNING KERN x1 : 0001 x0 : 
>> 809ca400
>> boot: logs: [   4.323183] WARNING KERN Call trace:
>> boot: logs: [   4.325593] WARNING KERN  
>> device_links_driver_bound+0x240/0x260
>> boot: logs: [   4.330301] WARNING KERN  driver_bound+0x70/0xd0
>> boot: logs: [   4.333740] WARNING KERN  device_bind_driver+0x50/0x60
>> boot: logs: [   4.337671] WARNING KERN  phy_attach_direct+0x258/0x2e0
>> boot: logs: [   4.341718] WARNING KERN  phylink_of_phy_connect+0x7c/0x140
>> boot: logs: [   4.346081] WARNING KERN  stmmac_open+0xb04/0xc70
>> boot: logs: [   4.349612] WARNING KERN  __dev_open+0xe0/0x190
>> boot: logs: [   4.352972] WARNING KERN  __dev_change_flags+0x16c/0x1b8
>> boot: logs: [   4.357081] WARNING KERN  dev_change_flags+0x20/0x60
>> boot: logs: [   4.360856] WARNING KERN  ip_auto_config+0x2a0/0xfe8
>> boot: logs: [   4.364633] WARNING KERN  do_one_initcall+0x58/0x1b8
>> boot: logs: [   4.368405] WARNING KERN  kernel_init_freeable+0x1ec/0x240
>> boot: logs: [   

Re: [PATCH v1 0/5] Enable fw_devlink=on by default

[Jon Hunter]

On 13/01/2021 21:29, Saravana Kannan wrote:

...

>> I am seeing the same problem on Tegra30 Cardhu A04 where several regulators
>> are continuously deferred and prevents the board from booting ...
>>
>> [2.518334] platform panel: probe deferral - supplier regulator@11 not 
>> ready
>>
>> [2.525503] platform regulator@1: probe deferral - supplier 4-002d not 
>> ready
>>
>> [2.533141] platform regulator@3: probe deferral - supplier regulator@101 
>> not ready
>>
>> [2.540856] platform regulator@5: probe deferral - supplier regulator@101 
>> not ready
>>
>> [2.548589] platform regulator@6: probe deferral - supplier regulator@101 
>> not ready
>>
>> [2.556316] platform regulator@7: probe deferral - supplier regulator@101 
>> not ready
>>
>> [2.564041] platform regulator@8: probe deferral - supplier regulator@101 
>> not ready
>>
>> [2.571743] platform regulator@9: probe deferral - supplier regulator@101 
>> not ready
>>
>> [2.579463] platform regulator@10: probe deferral - supplier 
>> regulator@101 not ready
>>
>> [2.587273] platform regulator@11: probe deferral - supplier 
>> regulator@101 not ready
>>
>> [2.595088] platform regulator@12: probe deferral - supplier 
>> regulator@104 not ready
>>
>> [2.603837] platform regulator@102: probe deferral - supplier 
>> regulator@104 not ready
>>
>> [2.611726] platform regulator@103: probe deferral - supplier 
>> regulator@104 not ready
>>
>> [2.620137] platform 3000.pcie: probe deferral - supplier regulator@5 not 
>> ready
> 
> Looks like this is not the whole log? Do you see any "wait for
> supplier" logs? That's what all these boot issues should boil down to.
> And as usual, pointer to DT for this board please.

Ah yes I see ...

 platform regulator@1: probe deferral - wait for supplier tps65911@2d

Yes the device-tree for this board can be found here [0]. Looks like
there is a circular dependency between the vddctrl_reg and vddcore_reg.
This is part of coupled regulators which have a two-way linkage [1]. So
this change appears to conflict with this.

Jon

[0]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/tegra30-cardhu-a04.dts
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/regulator/regulator.yaml#n129

-- 
nvpublic

Re: [Patch v2 0/4] Add Nvidia Tegra GPC-DMA driver

[Jon Hunter]

On 06/08/2020 08:30, Rajesh Gumasta wrote:
> Changes in patch v2:
> Addressed review comments in patch v1


Is there any update on this series? Would be good to get this upstream.

Jon

-- 
nvpublic

Re: [PATCH v1 0/5] Enable fw_devlink=on by default

[Jon Hunter]

On 13/01/2021 11:11, Marc Zyngier wrote:
> On 2021-01-07 20:05, Greg Kroah-Hartman wrote:
>> On Thu, Dec 17, 2020 at 07:16:58PM -0800, Saravana Kannan wrote:
>>> As discussed in LPC 2020, cyclic dependencies in firmware that couldn't
>>> be broken using logic was one of the last remaining reasons
>>> fw_devlink=on couldn't be set by default.
>>>
>>> This series changes fw_devlink so that when a cyclic dependency is found
>>> in firmware, the links between those devices fallback to permissive mode
>>> behavior. This way, the rest of the system still benefits from
>>> fw_devlink, but the ambiguous cases fallback to permissive mode.
>>>
>>> Setting fw_devlink=on by default brings a bunch of benefits (currently,
>>> only for systems with device tree firmware):
>>> * Significantly cuts down deferred probes.
>>> * Device probe is effectively attempted in graph order.
>>> * Makes it much easier to load drivers as modules without having to
>>>   worry about functional dependencies between modules (depmod is still
>>>   needed for symbol dependencies).
>>>
>>> Greg/Rafael,
>>>
>>> Can we get this pulled into 5.11-rc1 or -rc2 soon please? I expect to
>>> see some issues due to device drivers that aren't following best
>>> practices (they don't expose the device to driver core). Want to
>>> identify those early on and try to have them fixed before 5.11 release.
>>> See [1] for an example of such a case.
>>
>> Now queued up in my tree, will show up in linux-next in a few days,
>> let's see what breaks!  :)
>>
>> And it is scheduled for 5.12-rc1, not 5.11, sorry.
> 
> For the record, this breaks my rk3399 board, (NanoPC-T4) as no mass
> storage can be discovered (it lives on PCIe):
> 
> (initramfs) find /sys -name 'waiting_for_supplier'| xargs grep .| egrep
> -v ':0$'
> /sys/devices/platform/ff3d.i2c/i2c-4/4-0022/waiting_for_supplier:1
> /sys/devices/platform/f800.pcie/waiting_for_supplier:1
> /sys/devices/platform/fe32.mmc/waiting_for_supplier:1
> /sys/devices/platform/sdio-pwrseq/waiting_for_supplier:1
> /sys/devices/platform/ff3c.i2c/i2c-0/0-001b/waiting_for_supplier:1
> 
> Enabling the debug prints in device_links_check_suppliers(), I end up with
> the dump below (apologies for the size).


I am seeing the same problem on Tegra30 Cardhu A04 where several regulators
are continuously deferred and prevents the board from booting ...

[2.518334] platform panel: probe deferral - supplier regulator@11 not ready

[2.525503] platform regulator@1: probe deferral - supplier 4-002d not ready

[2.533141] platform regulator@3: probe deferral - supplier regulator@101 
not ready

[2.540856] platform regulator@5: probe deferral - supplier regulator@101 
not ready

[2.548589] platform regulator@6: probe deferral - supplier regulator@101 
not ready

[2.556316] platform regulator@7: probe deferral - supplier regulator@101 
not ready

[2.564041] platform regulator@8: probe deferral - supplier regulator@101 
not ready

[2.571743] platform regulator@9: probe deferral - supplier regulator@101 
not ready

[2.579463] platform regulator@10: probe deferral - supplier regulator@101 
not ready

[2.587273] platform regulator@11: probe deferral - supplier regulator@101 
not ready

[2.595088] platform regulator@12: probe deferral - supplier regulator@104 
not ready

[2.603837] platform regulator@102: probe deferral - supplier regulator@104 
not ready

[2.611726] platform regulator@103: probe deferral - supplier regulator@104 
not ready

[2.620137] platform 3000.pcie: probe deferral - supplier regulator@5 not 
ready


Cheers
Jon

-- 
nvpublic

Re: [PATCH v1 0/5] Enable fw_devlink=on by default

[Jon Hunter]

On 18/12/2020 03:16, Saravana Kannan wrote:
> As discussed in LPC 2020, cyclic dependencies in firmware that couldn't
> be broken using logic was one of the last remaining reasons
> fw_devlink=on couldn't be set by default.
> 
> This series changes fw_devlink so that when a cyclic dependency is found
> in firmware, the links between those devices fallback to permissive mode
> behavior. This way, the rest of the system still benefits from
> fw_devlink, but the ambiguous cases fallback to permissive mode.
> 
> Setting fw_devlink=on by default brings a bunch of benefits (currently,
> only for systems with device tree firmware):
> * Significantly cuts down deferred probes.
> * Device probe is effectively attempted in graph order.
> * Makes it much easier to load drivers as modules without having to
>   worry about functional dependencies between modules (depmod is still
>   needed for symbol dependencies).


One issue we have come across with this is the of_mdio.c driver. On
Tegra194 Jetson Xavier I am seeing the following ...

boot: logs: [   4.194791] WARNING KERN WARNING: CPU: 0 PID: 1 at 
/dvs/git/dirty/git-master_l4t-upstream/kernel/drivers/base/core.c:1189 
device_links_driver_bound+0x240/0x260
boot: logs: [   4.207683] WARNING KERN Modules linked in:
boot: logs: [   4.210691] WARNING KERN CPU: 0 PID: 1 Comm: swapper/0 Not 
tainted 5.11.0-rc3-next-20210112-gdf869cab4b35 #1
boot: logs: [   4.219221] WARNING KERN Hardware name: NVIDIA Jetson AGX 
Xavier Developer Kit (DT)
boot: logs: [   4.225628] WARNING KERN pstate: 8049 (Nzcv daif +PAN 
-UAO -TCO BTYPE=--)
boot: logs: [   4.231542] WARNING KERN pc : 
device_links_driver_bound+0x240/0x260
boot: logs: [   4.236587] WARNING KERN lr : 
device_links_driver_bound+0xf8/0x260
boot: logs: [   4.241560] WARNING KERN sp : 800011f4b980
boot: logs: [   4.244819] WARNING KERN x29: 800011f4b980 x28: 
8208a0a0
boot: logs: [   4.250051] WARNING KERN x27: 8208a080 x26: 

boot: logs: [   4.255271] WARNING KERN x25: 0003 x24: 
800011b99000
boot: logs: [   4.260489] WARNING KERN x23: 0001 x22: 
800011df14f0
boot: logs: [   4.265706] WARNING KERN x21: 800011f4b9f8 x20: 
800011df1000
boot: logs: [   4.270934] WARNING KERN x19: 8208a000 x18: 
0005
boot: logs: [   4.276166] WARNING KERN x17: 0007 x16: 
0001
boot: logs: [   4.281382] WARNING KERN x15: 80030c90 x14: 
805c9df8
boot: logs: [   4.286618] WARNING KERN x13:  x12: 
80030c90
boot: logs: [   4.291847] WARNING KERN x11: 805c9da8 x10: 
0040
boot: logs: [   4.297061] WARNING KERN x9 : 80030c98 x8 : 

boot: logs: [   4.302291] WARNING KERN x7 : 0009 x6 : 

boot: logs: [   4.307509] WARNING KERN x5 : 8010 x4 : 

boot: logs: [   4.312739] WARNING KERN x3 : 800011df1e38 x2 : 
80908c10
boot: logs: [   4.317956] WARNING KERN x1 : 0001 x0 : 
809ca400
boot: logs: [   4.323183] WARNING KERN Call trace:
boot: logs: [   4.325593] WARNING KERN  
device_links_driver_bound+0x240/0x260
boot: logs: [   4.330301] WARNING KERN  driver_bound+0x70/0xd0
boot: logs: [   4.333740] WARNING KERN  device_bind_driver+0x50/0x60
boot: logs: [   4.337671] WARNING KERN  phy_attach_direct+0x258/0x2e0
boot: logs: [   4.341718] WARNING KERN  phylink_of_phy_connect+0x7c/0x140
boot: logs: [   4.346081] WARNING KERN  stmmac_open+0xb04/0xc70
boot: logs: [   4.349612] WARNING KERN  __dev_open+0xe0/0x190
boot: logs: [   4.352972] WARNING KERN  __dev_change_flags+0x16c/0x1b8
boot: logs: [   4.357081] WARNING KERN  dev_change_flags+0x20/0x60
boot: logs: [   4.360856] WARNING KERN  ip_auto_config+0x2a0/0xfe8
boot: logs: [   4.364633] WARNING KERN  do_one_initcall+0x58/0x1b8
boot: logs: [   4.368405] WARNING KERN  kernel_init_freeable+0x1ec/0x240
boot: logs: [   4.372698] WARNING KERN  kernel_init+0x10/0x110
boot: logs: [   4.376130] WARNING KERN  ret_from_fork+0x10/0x18


So looking at this change does this mean that the of_mdio needs to be
converted to a proper driver? I would have thought that this will be
seen on several platforms.

Cheers
Jon

-- 
nvpublic

Re: [PATCH 2/2] ALSA: hda/tegra: fix tegra-hda on tegra30 soc

[Jon Hunter]

On 08/01/2021 10:54, Jon Hunter wrote:
> 
> On 08/01/2021 08:00, Sameer Pujar wrote:
> 
> ...
> 
>>>>> Signed-off-by: Peter Geis 
>>>>> Tested-by: Ion Agorria 
>>>>> ---
>>>>>    sound/pci/hda/hda_tegra.c | 3 +--
>>>>>    1 file changed, 1 insertion(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/sound/pci/hda/hda_tegra.c b/sound/pci/hda/hda_tegra.c
>>>>> index 70164d1428d4..f8d61e677a09 100644
>>>>> --- a/sound/pci/hda/hda_tegra.c
>>>>> +++ b/sound/pci/hda/hda_tegra.c
>>>>> @@ -388,8 +388,7 @@ static int hda_tegra_first_init(struct azx
>>>>> *chip, struct platform_device *pdev)
>>>>>    * in powers of 2, next available ratio is 16 which can be
>>>>>    * used as a limiting factor here.
>>>>>    */
>>>>> -   if (of_device_is_compatible(np, "nvidia,tegra194-hda"))
>>>>> -   chip->bus.core.sdo_limit = 16;
>>>>> +   chip->bus.core.sdo_limit = 16;
>>>> Future Tegra chips address this problem and hence cannot be enforced by
>>>> default. May be we can have like below:
>>>>
>>>> if (of_device_is_compatible(np, "nvidia,tegra30-hda"))
>>>> chip->bus.core.sdo_limit = 16;
>>>>
>>> It will need to be a bit more complicated than that, since the
>>> tegra186 and tegra210 device trees have "nvidia,tegra30-hda" as a
>>> fallback.
>>> Looking at the generation map, tegra30-hda can be the fallback for the
>>> broken implementation and tegra210-hda can be the fallback for the
>>> working implementation.
>>> Does that work for you?
>>
>> As per above explanation, it is fine to apply the workaround for
>> Tegra210/186 as well. So it simplifies things for all existing chips.
> 
> 
> FYI ... we now have minimal support for Tegra234 in upstream that should
> not require this. Given that the Tegra234 device-tree does not include
> support for HDA yet, I think it is fine to apply this as-is. However,
> once we do add support for Tegra234 HDA, then we should ensure that this
> is not applied. So that said ...
> 
> Reviewed-by: Jon Hunter 


Sorry I was chatting with Sameer offline and we think if we just switch
the test to the following then this will take care of Tegra234 when we
add it ...

if (of_device_is_compatible(np, "nvidia,tegra30-hda"))

Peter, would you be able to send a V2 with this?

Thanks!
Jon

-- 
nvpublic

Re: [PATCH 2/2] ALSA: hda/tegra: fix tegra-hda on tegra30 soc

[Jon Hunter]

On 08/01/2021 08:00, Sameer Pujar wrote:

...

>>>> Signed-off-by: Peter Geis 
>>>> Tested-by: Ion Agorria 
>>>> ---
>>>>    sound/pci/hda/hda_tegra.c | 3 +--
>>>>    1 file changed, 1 insertion(+), 2 deletions(-)
>>>>
>>>> diff --git a/sound/pci/hda/hda_tegra.c b/sound/pci/hda/hda_tegra.c
>>>> index 70164d1428d4..f8d61e677a09 100644
>>>> --- a/sound/pci/hda/hda_tegra.c
>>>> +++ b/sound/pci/hda/hda_tegra.c
>>>> @@ -388,8 +388,7 @@ static int hda_tegra_first_init(struct azx
>>>> *chip, struct platform_device *pdev)
>>>>    * in powers of 2, next available ratio is 16 which can be
>>>>    * used as a limiting factor here.
>>>>    */
>>>> -   if (of_device_is_compatible(np, "nvidia,tegra194-hda"))
>>>> -   chip->bus.core.sdo_limit = 16;
>>>> +   chip->bus.core.sdo_limit = 16;
>>> Future Tegra chips address this problem and hence cannot be enforced by
>>> default. May be we can have like below:
>>>
>>> if (of_device_is_compatible(np, "nvidia,tegra30-hda"))
>>> chip->bus.core.sdo_limit = 16;
>>>
>> It will need to be a bit more complicated than that, since the
>> tegra186 and tegra210 device trees have "nvidia,tegra30-hda" as a
>> fallback.
>> Looking at the generation map, tegra30-hda can be the fallback for the
>> broken implementation and tegra210-hda can be the fallback for the
>> working implementation.
>> Does that work for you?
> 
> As per above explanation, it is fine to apply the workaround for
> Tegra210/186 as well. So it simplifies things for all existing chips.


FYI ... we now have minimal support for Tegra234 in upstream that should
not require this. Given that the Tegra234 device-tree does not include
support for HDA yet, I think it is fine to apply this as-is. However,
once we do add support for Tegra234 HDA, then we should ensure that this
is not applied. So that said ...

Reviewed-by: Jon Hunter 

Cheers
Jon

-- 
nvpublic

Re: [PATCH] arm64: tegra: Add power-domain for Tegra210 HDA

[Jon Hunter]

On 07/01/2021 05:06, Sameer Pujar wrote:
> HDA initialization is failing occasionally on Tegra210 and following
> print is observed in the boot log. Because of this probe() fails and
> no sound card is registered.
> 
>   [16.800802] tegra-hda 7003.hda: no codecs found!
> 
> Codecs request a state change and enumeration by the controller. In
> failure cases this does not seem to happen as STATETS register reads 0.
> 
> The problem seems to be related to the HDA codec dependency on SOR
> power domain. If it is gated during HDA probe then the failure is
> observed. Building Tegra HDA driver into kernel image avoids this
> failure but does not completely address the dependency part. Fix this
> problem by adding 'power-domains' DT property for Tegra210 HDA. Note
> that Tegra186 and Tegra194 HDA do this already.
> 
> Fixes: 742af7e7a0a1 ("arm64: tegra: Add Tegra210 support")
> Depends-on: 96d1f078ff0 ("arm64: tegra: Add SOR power-domain for Tegra210")
> Cc: 
> Signed-off-by: Sameer Pujar 
> ---
>  arch/arm64/boot/dts/nvidia/tegra210.dtsi | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi 
> b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
> index 4fbf8c1..fd33b4d 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
> @@ -997,6 +997,7 @@
><_car 128>, /* hda2hdmi */
><_car 111>; /* hda2codec_2x */
>   reset-names = "hda", "hda2hdmi", "hda2codec_2x";
> + power-domains = <_sor>;
>   status = "disabled";
>   };

Thanks!

Acked-by: Jon Hunter 

Cheers
Jon

-- 
nvpublic

Re: [PATCH 1/2] clk: tegra30: Add hda clock default rates to clock driver

[Jon Hunter]

On 25/12/2020 01:20, Peter Geis wrote:
> Current implementation defaults the hda clocks to clk_m.
> This causes hda to run too slow to operate correctly.
> Fix this by defaulting to pll_p and setting the frequency to the correct rate.
> 
> This matches upstream t124 and downstream t30.
> 
> Signed-off-by: Peter Geis 
> Tested-by: Ion Agorria 
> ---
>  drivers/clk/tegra/clk-tegra30.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c
> index 37244a7e68c2..9cf249c344d9 100644
> --- a/drivers/clk/tegra/clk-tegra30.c
> +++ b/drivers/clk/tegra/clk-tegra30.c
> @@ -1256,6 +1256,8 @@ static struct tegra_clk_init_table init_table[] 
> __initdata = {
>   { TEGRA30_CLK_I2S3_SYNC, TEGRA30_CLK_CLK_MAX, 2400, 0 },
>   { TEGRA30_CLK_I2S4_SYNC, TEGRA30_CLK_CLK_MAX, 2400, 0 },
>   { TEGRA30_CLK_VIMCLK_SYNC, TEGRA30_CLK_CLK_MAX, 2400, 0 },
> + { TEGRA30_CLK_HDA, TEGRA30_CLK_PLL_P, 10200, 0 },
> + { TEGRA30_CLK_HDA2CODEC_2X, TEGRA30_CLK_PLL_P, 4800, 0 },
>   /* must be the last entry */
>   { TEGRA30_CLK_CLK_MAX, TEGRA30_CLK_CLK_MAX, 0, 0 },
>  };


This looks good to me. So ...

Acked-by: Jon Hunter 

Cheers
Jon

-- 
nvpublic

Re: [PATCH 5.10 00/16] 5.10.2-rc1 review

[Jon Hunter]

On 19/12/2020 12:57, Greg Kroah-Hartman wrote:
> This is the start of the stable review cycle for the 5.10.2 release.
> There are 16 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.
> 
> Responses should be made by Mon, 21 Dec 2020 12:53:29 +.
> Anything received after that time might be too late.
> 
> The whole patch series can be found in one patch at:
>   
> https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.10.2-rc1.gz
> or in the git tree and branch at:
>   
> git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git 
> linux-5.10.y
> and the diffstat can be found below.
> 
> thanks,
> 
> greg k-h


Test results for stable-v5.10:
12 builds:  12 pass, 0 fail
26 boots:   26 pass, 0 fail
64 tests:   63 pass, 1 fail

Linux version:  5.10.2-rc1-gc96cfd687a3f
Boards tested:  tegra124-jetson-tk1, tegra186-p2771-,
tegra194-p2972-, tegra20-ventana,
tegra210-p2371-2180, tegra210-p3450-,
tegra30-cardhu-a04

Test failures:  tegra194-p2972-: boot.py


Same warning failure as before. The fix for this is now in the mainline
if you would like to pick it up ...

commit c9f64d1fc101c64ea2be1b2e562b4395127befc9
Author: Thierry Reding 
Date:   Tue Nov 10 08:37:57 2020 +0100

net: ipconfig: Avoid spurious blank lines in boot log


Otherwise ...

Tested-by: Jon Hunter 

Cheers
Jon

-- 
nvpublic

Re: [PATCH] gcc-plugins: simplify GCC plugin-dev capability test

[Jon Hunter]

On 18/12/2020 17:54, Linus Torvalds wrote:
> On Fri, Dec 18, 2020 at 7:33 AM Jon Hunter  wrote:
>>
>> However, if you are saying that this is a problem/bug with our builders,
>> then of course we will have to get this fixed.
> 
> This seems to be a package dependency problem with the gcc plugins -
> they clearly want libgmp, but apparently the package hasn't specified
> that dependency.
> 
> If this turns out to be a big problem, I guess we can't simplify the
> plugin check after all.
> 
> We historically just disabled gcc-plugins if that header didn't build,
> which obviously meant that it "worked" for people, but it also means
> that clearly the coverage can't have been as good as it could/should
> be.
> 
> So if it's as simple as just installing the GNU multiprecision
> libraries ("gmp-devel" on most rpm-based systems, "libgmp-dev" on most
> debian systems), then I think that's the right thing to do. You'll get
> a working build again, and equally importantly, your build servers
> will actually do a better job of covering the different build options.


Thanks. I have reported this issue to the team that administers the
builders. So hopefully, they will install the necessary packages for us
now.

Cheers
Jon

-- 
nvpublic

Re: [PATCH] gcc-plugins: simplify GCC plugin-dev capability test

[Jon Hunter]

On 18/12/2020 15:42, Masahiro Yamada wrote:

...

>> However, if you are saying that this is a problem/bug with our builders,
>> then of course we will have to get this fixed.
>>
> 
> 
> Yes, please do so.
> 
> 
> Kconfig evaluates $(CC) capabilities, and
> hides CONFIG options it cannot support.
> 
> 
> In contrast, we do not do that for $(HOSTCC)
> capabilities because it is just a matter of some
> missing packages.
> 
> 
> For example, if you enable CONFIG_SYSTEM_TRUSTED_KEYRING
> and fail to build scripts/extrace-cert.c
> due to missing ,
> you need to install the openssl dev package.
> 
> It is the same pattern.


OK, thanks for confirming. We will get this fixed.

Cheers Jon

-- 
nvpublic

Re: [PATCH] gcc-plugins: simplify GCC plugin-dev capability test

[Jon Hunter]

On 18/12/2020 15:12, Jon Hunter wrote:
> 
> On 18/12/2020 15:09, Marek Szyprowski wrote:
>>
>> On 18.12.2020 16:03, Jon Hunter wrote:
>>> On 18/12/2020 10:05, Marek Szyprowski wrote:
>>>> On 18.12.2020 10:43, Masahiro Yamada wrote:
>>>>> On Fri, Dec 18, 2020 at 4:58 PM Marek Szyprowski
>>>>>  wrote:
>>>>>> On 03.12.2020 13:57, Masahiro Yamada wrote:
>>>>>>> Linus pointed out a third of the time in the Kconfig parse stage comes
>>>>>>> from the single invocation of cc1plus in scripts/gcc-plugin.sh [1],
>>>>>>> and directly testing plugin-version.h for existence cuts down the
>>>>>>> overhead a lot. [2]
>>>>>>>
>>>>>>> This commit takes one step further to kill the build test entirely.
>>>>>>>
>>>>>>> The small piece of code was probably intended to test the C++ designated
>>>>>>> initializer, which was not supported until C++20.
>>>>>>>
>>>>>>> In fact, with -pedantic option given, both GCC and Clang emit a warning.
>>>>>>>
>>>>>>> $ echo 'class test { public: int test; } test = { .test = 1 };' | g++ 
>>>>>>> -x c++ -pedantic - -fsyntax-only
>>>>>>> :1:43: warning: C++ designated initializers only available with 
>>>>>>> '-std=c++2a' or '-std=gnu++2a' [-Wpedantic]
>>>>>>> $ echo 'class test { public: int test; } test = { .test = 1 };' | 
>>>>>>> clang++ -x c++ -pedantic - -fsyntax-only
>>>>>>> :1:43: warning: designated initializers are a C++20 extension 
>>>>>>> [-Wc++20-designator]
>>>>>>> class test { public: int test; } test = { .test = 1 };
>>>>>>>  ^
>>>>>>> 1 warning generated.
>>>>>>>
>>>>>>> Otherwise, modern C++ compilers should be able to build the code, and
>>>>>>> hopefully skipping this test should not make any practical problem.
>>>>>>>
>>>>>>> Checking the existence of plugin-version.h is still needed to ensure
>>>>>>> the plugin-dev package is installed. The test code is now small enough
>>>>>>> to be embedded in scripts/gcc-plugins/Kconfig.
>>>>>>>
>>>>>>> [1] 
>>>>>>> https://protect2.fireeye.com/v1/url?k=03db90e1-5c40a828-03da1bae-0cc47a336fae-4cc36f5830aeb78d=1=dfdc1cf9-82d6-4ca5-b35d-1782e918bde3=https%3A%2F%2Flore.kernel.org%2Flkml%2FCAHk-%3DwjU4DCuwQ4pXshRbwDCUQB31ScaeuDo1tjoZ0_PjhLHzQ%40mail.gmail.com%2F
>>>>>>> [2] 
>>>>>>> https://protect2.fireeye.com/v1/url?k=965b670a-c9c05fc3-965aec45-0cc47a336fae-e34339513ff747c0=1=dfdc1cf9-82d6-4ca5-b35d-1782e918bde3=https%3A%2F%2Flore.kernel.org%2Flkml%2FCAHk-%3DwhK0aQxs6Q5ijJmYF1n2ch8cVFSUzU5yUM_HOjig%3D%2Bvnw%40mail.gmail.com%2F
>>>>>>>
>>>>>>> Reported-by: Linus Torvalds 
>>>>>>> Signed-off-by: Masahiro Yamada 
>>>>>> This patch landed in linux next-20201217 as commit 1e860048c53e
>>>>>> ("gcc-plugins: simplify GCC plugin-dev capability test").
>>>>>>
>>>>>> It causes a build break with my tests setup, but I'm not sure weather it
>>>>>> is really an issue of this commit or a toolchain I use. However I've
>>>>>> checked various versions of the gcc cross-compilers released by Linaro
>>>>>> at 
>>>>>> https://protect2.fireeye.com/v1/url?k=053727b6-5aac1f7f-0536acf9-0cc47a336fae-5bd799e7ce6b1b9b=1=dfdc1cf9-82d6-4ca5-b35d-1782e918bde3=https%3A%2F%2Freleases.linaro.org%2Fcomponents%2Ftoolchain%2Fbinaries%2F
>>>>>>  and all
>>>>>> fails with the same error:
>>>>>>
>>>>>> $ make ARCH=arm
>>>>>> CROSS_COMPILE=../../cross/gcc-arm-10.2-2020.11-x86_64-arm-none-eabi/bin/arm-none-eabi-
>>>>>> zImage
>>>>>>  HOSTCXX scripts/gcc-plugins/arm_ssp_per_task_plugin.so
>>>>>> In file included from
>>>>>> /home/mszyprow/dev/cross/gcc-arm-10.2-2020.11-x86_64-arm-none-eabi/bin/../lib/gcc/arm-none-eabi/10.2.1/plugin/include/gcc-plugin.h:28:0,
>>>>>> from scripts/gcc-plugins/gcc-common.h:7,
>>>>>> from scripts/gcc-plugins/arm_ssp_per_task_plugin.c:3:
>>>

Re: [PATCH] gcc-plugins: simplify GCC plugin-dev capability test

[Jon Hunter]

On 18/12/2020 15:09, Marek Szyprowski wrote:
> 
> On 18.12.2020 16:03, Jon Hunter wrote:
>> On 18/12/2020 10:05, Marek Szyprowski wrote:
>>> On 18.12.2020 10:43, Masahiro Yamada wrote:
>>>> On Fri, Dec 18, 2020 at 4:58 PM Marek Szyprowski
>>>>  wrote:
>>>>> On 03.12.2020 13:57, Masahiro Yamada wrote:
>>>>>> Linus pointed out a third of the time in the Kconfig parse stage comes
>>>>>> from the single invocation of cc1plus in scripts/gcc-plugin.sh [1],
>>>>>> and directly testing plugin-version.h for existence cuts down the
>>>>>> overhead a lot. [2]
>>>>>>
>>>>>> This commit takes one step further to kill the build test entirely.
>>>>>>
>>>>>> The small piece of code was probably intended to test the C++ designated
>>>>>> initializer, which was not supported until C++20.
>>>>>>
>>>>>> In fact, with -pedantic option given, both GCC and Clang emit a warning.
>>>>>>
>>>>>> $ echo 'class test { public: int test; } test = { .test = 1 };' | g++ -x 
>>>>>> c++ -pedantic - -fsyntax-only
>>>>>> :1:43: warning: C++ designated initializers only available with 
>>>>>> '-std=c++2a' or '-std=gnu++2a' [-Wpedantic]
>>>>>> $ echo 'class test { public: int test; } test = { .test = 1 };' | 
>>>>>> clang++ -x c++ -pedantic - -fsyntax-only
>>>>>> :1:43: warning: designated initializers are a C++20 extension 
>>>>>> [-Wc++20-designator]
>>>>>> class test { public: int test; } test = { .test = 1 };
>>>>>>  ^
>>>>>> 1 warning generated.
>>>>>>
>>>>>> Otherwise, modern C++ compilers should be able to build the code, and
>>>>>> hopefully skipping this test should not make any practical problem.
>>>>>>
>>>>>> Checking the existence of plugin-version.h is still needed to ensure
>>>>>> the plugin-dev package is installed. The test code is now small enough
>>>>>> to be embedded in scripts/gcc-plugins/Kconfig.
>>>>>>
>>>>>> [1] 
>>>>>> https://protect2.fireeye.com/v1/url?k=03db90e1-5c40a828-03da1bae-0cc47a336fae-4cc36f5830aeb78d=1=dfdc1cf9-82d6-4ca5-b35d-1782e918bde3=https%3A%2F%2Flore.kernel.org%2Flkml%2FCAHk-%3DwjU4DCuwQ4pXshRbwDCUQB31ScaeuDo1tjoZ0_PjhLHzQ%40mail.gmail.com%2F
>>>>>> [2] 
>>>>>> https://protect2.fireeye.com/v1/url?k=965b670a-c9c05fc3-965aec45-0cc47a336fae-e34339513ff747c0=1=dfdc1cf9-82d6-4ca5-b35d-1782e918bde3=https%3A%2F%2Flore.kernel.org%2Flkml%2FCAHk-%3DwhK0aQxs6Q5ijJmYF1n2ch8cVFSUzU5yUM_HOjig%3D%2Bvnw%40mail.gmail.com%2F
>>>>>>
>>>>>> Reported-by: Linus Torvalds 
>>>>>> Signed-off-by: Masahiro Yamada 
>>>>> This patch landed in linux next-20201217 as commit 1e860048c53e
>>>>> ("gcc-plugins: simplify GCC plugin-dev capability test").
>>>>>
>>>>> It causes a build break with my tests setup, but I'm not sure weather it
>>>>> is really an issue of this commit or a toolchain I use. However I've
>>>>> checked various versions of the gcc cross-compilers released by Linaro
>>>>> at 
>>>>> https://protect2.fireeye.com/v1/url?k=053727b6-5aac1f7f-0536acf9-0cc47a336fae-5bd799e7ce6b1b9b=1=dfdc1cf9-82d6-4ca5-b35d-1782e918bde3=https%3A%2F%2Freleases.linaro.org%2Fcomponents%2Ftoolchain%2Fbinaries%2F
>>>>>  and all
>>>>> fails with the same error:
>>>>>
>>>>> $ make ARCH=arm
>>>>> CROSS_COMPILE=../../cross/gcc-arm-10.2-2020.11-x86_64-arm-none-eabi/bin/arm-none-eabi-
>>>>> zImage
>>>>>  HOSTCXX scripts/gcc-plugins/arm_ssp_per_task_plugin.so
>>>>> In file included from
>>>>> /home/mszyprow/dev/cross/gcc-arm-10.2-2020.11-x86_64-arm-none-eabi/bin/../lib/gcc/arm-none-eabi/10.2.1/plugin/include/gcc-plugin.h:28:0,
>>>>> from scripts/gcc-plugins/gcc-common.h:7,
>>>>> from scripts/gcc-plugins/arm_ssp_per_task_plugin.c:3:
>>>>> /home/mszyprow/dev/cross/gcc-arm-10.2-2020.11-x86_64-arm-none-eabi/bin/../lib/gcc/arm-none-eabi/10.2.1/plugin/include/system.h:687:10:
>>>>> fatal error: gmp.h: No such file or directory
>>>>> #include 
>>>>>  ^~~~

Re: [PATCH] gcc-plugins: simplify GCC plugin-dev capability test

[Jon Hunter]

On 18/12/2020 10:05, Marek Szyprowski wrote:
> On 18.12.2020 10:43, Masahiro Yamada wrote:
>> On Fri, Dec 18, 2020 at 4:58 PM Marek Szyprowski
>>  wrote:
>>> On 03.12.2020 13:57, Masahiro Yamada wrote:
 Linus pointed out a third of the time in the Kconfig parse stage comes
 from the single invocation of cc1plus in scripts/gcc-plugin.sh [1],
 and directly testing plugin-version.h for existence cuts down the
 overhead a lot. [2]

 This commit takes one step further to kill the build test entirely.

 The small piece of code was probably intended to test the C++ designated
 initializer, which was not supported until C++20.

 In fact, with -pedantic option given, both GCC and Clang emit a warning.

 $ echo 'class test { public: int test; } test = { .test = 1 };' | g++ -x 
 c++ -pedantic - -fsyntax-only
 :1:43: warning: C++ designated initializers only available with 
 '-std=c++2a' or '-std=gnu++2a' [-Wpedantic]
 $ echo 'class test { public: int test; } test = { .test = 1 };' | clang++ 
 -x c++ -pedantic - -fsyntax-only
 :1:43: warning: designated initializers are a C++20 extension 
 [-Wc++20-designator]
 class test { public: int test; } test = { .test = 1 };
 ^
 1 warning generated.

 Otherwise, modern C++ compilers should be able to build the code, and
 hopefully skipping this test should not make any practical problem.

 Checking the existence of plugin-version.h is still needed to ensure
 the plugin-dev package is installed. The test code is now small enough
 to be embedded in scripts/gcc-plugins/Kconfig.

 [1] 
 https://protect2.fireeye.com/v1/url?k=03db90e1-5c40a828-03da1bae-0cc47a336fae-4cc36f5830aeb78d=1=dfdc1cf9-82d6-4ca5-b35d-1782e918bde3=https%3A%2F%2Flore.kernel.org%2Flkml%2FCAHk-%3DwjU4DCuwQ4pXshRbwDCUQB31ScaeuDo1tjoZ0_PjhLHzQ%40mail.gmail.com%2F
 [2] 
 https://protect2.fireeye.com/v1/url?k=965b670a-c9c05fc3-965aec45-0cc47a336fae-e34339513ff747c0=1=dfdc1cf9-82d6-4ca5-b35d-1782e918bde3=https%3A%2F%2Flore.kernel.org%2Flkml%2FCAHk-%3DwhK0aQxs6Q5ijJmYF1n2ch8cVFSUzU5yUM_HOjig%3D%2Bvnw%40mail.gmail.com%2F

 Reported-by: Linus Torvalds 
 Signed-off-by: Masahiro Yamada 
>>> This patch landed in linux next-20201217 as commit 1e860048c53e
>>> ("gcc-plugins: simplify GCC plugin-dev capability test").
>>>
>>> It causes a build break with my tests setup, but I'm not sure weather it
>>> is really an issue of this commit or a toolchain I use. However I've
>>> checked various versions of the gcc cross-compilers released by Linaro
>>> at 
>>> https://protect2.fireeye.com/v1/url?k=053727b6-5aac1f7f-0536acf9-0cc47a336fae-5bd799e7ce6b1b9b=1=dfdc1cf9-82d6-4ca5-b35d-1782e918bde3=https%3A%2F%2Freleases.linaro.org%2Fcomponents%2Ftoolchain%2Fbinaries%2F
>>>  and all
>>> fails with the same error:
>>>
>>> $ make ARCH=arm
>>> CROSS_COMPILE=../../cross/gcc-arm-10.2-2020.11-x86_64-arm-none-eabi/bin/arm-none-eabi-
>>> zImage
>>> HOSTCXX scripts/gcc-plugins/arm_ssp_per_task_plugin.so
>>> In file included from
>>> /home/mszyprow/dev/cross/gcc-arm-10.2-2020.11-x86_64-arm-none-eabi/bin/../lib/gcc/arm-none-eabi/10.2.1/plugin/include/gcc-plugin.h:28:0,
>>>from scripts/gcc-plugins/gcc-common.h:7,
>>>from scripts/gcc-plugins/arm_ssp_per_task_plugin.c:3:
>>> /home/mszyprow/dev/cross/gcc-arm-10.2-2020.11-x86_64-arm-none-eabi/bin/../lib/gcc/arm-none-eabi/10.2.1/plugin/include/system.h:687:10:
>>> fatal error: gmp.h: No such file or directory
>>>#include 
>>> ^~~
>>> compilation terminated.
>>> scripts/gcc-plugins/Makefile:47: recipe for target
>>> 'scripts/gcc-plugins/arm_ssp_per_task_plugin.so' failed
>>> make[2]: *** [scripts/gcc-plugins/arm_ssp_per_task_plugin.so] Error 1
>>> scripts/Makefile.build:496: recipe for target 'scripts/gcc-plugins' failed
>>> make[1]: *** [scripts/gcc-plugins] Error 2
>>> Makefile:1190: recipe for target 'scripts' failed
>>> make: *** [scripts] Error 2
>>>
>>> Compilation works if I use the cross-gcc provided by
>>> gcc-7-arm-linux-gnueabi/gcc-arm-linux-gnueabi Ubuntu packages, which is:
>>>
>>> $ arm-linux-gnueabi-gcc --version
>>> arm-linux-gnueabi-gcc (Ubuntu/Linaro 7.5.0-3ubuntu1~18.04) 7.5.0
>>>
>>
>> I can compile gcc-plugins with Linaro toolchians.
>>
>> The version of mine is this:
>>
>> masahiro@oscar:~/ref/linux-next$
>> ~/tools/arm-linaro-7.5/bin/arm-linux-gnueabihf-gcc --version
>> arm-linux-gnueabihf-gcc (Linaro GCC 7.5-2019.12) 7.5.0
>> Copyright (C) 2017 Free Software Foundation, Inc.
>> This is free software; see the source for copying conditions.  There is NO
>> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>>
>>
>>
>>
>> Maybe, it depends on the host environment?
>>
>>
>> Please try this:
>>
>> $ sudo apt install libgmp-dev
> 
> Indeed, it was missing on my setup. Sorry for the noise.


So this change 

Re: [PATCH 5.10 0/2] 5.10.1-rc1 review

[Jon Hunter]

Hi Greg,

On 14/12/2020 17:06, Greg Kroah-Hartman wrote:
> This is the start of the stable review cycle for the 5.10.1 release.
> There are 2 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.
> 
> Responses should be made by Monday, 14 Dec 2020 18:04:42 +.
> Anything received after that time might be too late.
> 
> The whole patch series can be found in one patch at:
>   
> https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.10.1-rc1.gz
> or in the git tree and branch at:
>   
> git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git 
> linux-5.10.y
> and the diffstat can be found below.
> 
> thanks,
> 
> greg k-h


Test results for stable-v5.10:
15 builds:  15 pass, 0 fail
26 boots:   26 pass, 0 fail
64 tests:   63 pass, 1 fail

Linux version:  5.10.1-g841fca5a32cc
Boards tested:  tegra124-jetson-tk1, tegra186-p2771-,
tegra194-p2972-, tegra20-ventana,
tegra210-p2371-2180, tegra210-p3450-,
tegra30-cardhu-a04

Test failures:  tegra194-p2972-: boot.py


Some changes in v5.10 exposed a minor issue in the kernel that can cause
a blank line warning to be seen. The above test is failing due to this
blank warning. Thierry has posted a fix for this [0] and should be
merged for v5.11. It was not tagged for stable, but if you OK to pull
this in once in the mainline I can send a request. Otherwise all looks
good, so ...

Tested-by: Jon Hunter 

Cheers
Jon

[0] https://lore.kernel.org/patchwork/patch/1336079/

-- 
nvpublic

Re: [PATCH] mm/memblock:use a more appropriate order calculation when free memblock pages

[Jon Hunter]

On 04/12/2020 16:07, Marek Szyprowski wrote:
> Hi All,
> 
> On 04.12.2020 14:42, Qian Cai wrote:
>> On Thu, 2020-12-03 at 23:23 +0800, carver4...@163.com wrote:
>>> From: Hailong Liu 
>>>
>>> When system in the booting stage, pages span from [start, end] of a memblock
>>> are freed to buddy in a order as large as possible (less than MAX_ORDER) at
>>> first, then decrease gradually to a proper order(less than end) in a loop.
>>>
>>> However, *min(MAX_ORDER - 1UL, __ffs(start))* can not get the largest order
>>> in some cases.
>>> Instead, *__ffs(end - start)* may be more appropriate and meaningful.
>>>
>>> Signed-off-by: Hailong Liu 
>> Reverting this commit on the top of today's linux-next fixed boot crashes on
>> multiple NUMA systems.
> 
> I confirm. Reverting commit 4df001639c84 ("mm/memblock: use a more 
> appropriate order calculation when free memblock pages") on top of linux 
> next-20201204 fixed booting of my ARM32bit test systems.


FWIW, I also confirm that this is causing several 32-bit Tegra platforms
to crash on boot and reverting this fixes the problem.

Jon

-- 
nvpublic

Re: [PATCH v1 3/7] spi: qspi-tegra: Add support for Tegra210 QSPI controller

[Jon Hunter]

On 01/12/2020 21:12, Sowjanya Komatineni wrote:
> Tegra SoC has a Quad SPI controller starting from Tegra210.
> 
> This patch adds support for Tegra210 QSPI controller.
> 
> Signed-off-by: Sowjanya Komatineni 
> ---
>  drivers/spi/Kconfig  |9 +
>  drivers/spi/Makefile |1 +
>  drivers/spi/qspi-tegra.c | 1418 
> ++
>  3 files changed, 1428 insertions(+)
>  create mode 100644 drivers/spi/qspi-tegra.c
> 
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index 3fd16b7..1a021e8 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -844,6 +844,15 @@ config SPI_MXS
>   help
> SPI driver for Freescale MXS devices.
>  
> +config QSPI_TEGRA
> + tristate "Nvidia Tegra QSPI Controller"
> + depends on (ARCH_TEGRA && TEGRA20_APB_DMA) || COMPILE_TEST

I assume that the dependency on the APBDMA is for Tegra210. Does it work
on Tegra210 without the DMA? I am wondering if this is a dependency?

> +static void tegra_qspi_deinit_dma_param(struct tegra_qspi_data *tqspi,
> + bool dma_to_memory)
> +{
> + u32 *dma_buf;
> + dma_addr_t dma_phys;
> + struct dma_chan *dma_chan;
> +
> + if (dma_to_memory) {
> + dma_buf = tqspi->rx_dma_buf;
> + dma_chan = tqspi->rx_dma_chan;
> + dma_phys = tqspi->rx_dma_phys;
> + tqspi->rx_dma_chan = NULL;
> + tqspi->rx_dma_buf = NULL;
> + } else {
> + dma_buf = tqspi->tx_dma_buf;
> + dma_chan = tqspi->tx_dma_chan;
> + dma_phys = tqspi->tx_dma_phys;
> + tqspi->tx_dma_buf = NULL;
> + tqspi->tx_dma_chan = NULL;
> + }
> + if (!dma_chan)
> + return;

The above seemed odd to me at first, but I guess if a device does not
support DMA yet, then this will be NULL. However, would it be clearer to
just ...

if (!tqspi->use_dma)
return;

You could also do this right at the beginning of the function.

> +static struct tegra_qspi_client_data
> + *tegra_qspi_parse_cdata_dt(struct spi_device *spi)
> +{
> + struct tegra_qspi_client_data *cdata;
> + struct device_node *slave_np;
> +
> + slave_np = spi->dev.of_node;
> + if (!slave_np) {

This test should not be necessary as we only support device-tree.

> + dev_dbg(>dev, "device node not found\n");
> + return NULL;
> + }
> +
> + cdata = kzalloc(sizeof(*cdata), GFP_KERNEL);
> + if (!cdata)
> + return NULL;
> +
> + of_property_read_u32(slave_np, "nvidia,tx-clk-tap-delay",
> +  >tx_clk_tap_delay);
> + of_property_read_u32(slave_np, "nvidia,rx-clk-tap-delay",
> +  >rx_clk_tap_delay);
> + return cdata;
> +}
> +
> +static void tegra_qspi_cleanup(struct spi_device *spi)
> +{
> + struct tegra_qspi_client_data *cdata = spi->controller_data;
> +
> + spi->controller_data = NULL;
> + if (spi->dev.of_node)
> + kfree(cdata);
> +}
> +
> +static int tegra_qspi_setup(struct spi_device *spi)
> +{
> + struct tegra_qspi_data *tqspi = spi_master_get_devdata(spi->master);
> + struct tegra_qspi_client_data *cdata = spi->controller_data;
> + u32 tx_tap = 0, rx_tap = 0;
> + u32 val;
> + unsigned long flags;
> + int ret;
> +
> + dev_dbg(>dev, "setup %d bpw, %scpol, %scpha, %dHz\n",
> + spi->bits_per_word,
> + spi->mode & SPI_CPOL ? "" : "~",
> + spi->mode & SPI_CPHA ? "" : "~",
> + spi->max_speed_hz);
> +
> + if (!cdata) {
> + cdata = tegra_qspi_parse_cdata_dt(spi);
> + spi->controller_data = cdata;
> + }
> +
> + ret = pm_runtime_get_sync(tqspi->dev);
> + if (ret < 0) {
> + dev_err(tqspi->dev, "runtime resume failed: %d\n", ret);
> + if (cdata)
> + tegra_qspi_cleanup(spi);
> + return ret;
> + }

Does it simplify the code to do the pm_runtime_get_sync() before the
parsing of the cdata?

> +static int tegra_qspi_probe(struct platform_device *pdev)
> +{
> + struct spi_master   *master;
> + struct tegra_qspi_data  *tqspi;
> + struct resource *r;
> + int ret, qspi_irq;
> + int bus_num;
> +
> + master = spi_alloc_master(>dev, sizeof(*tqspi));
> + if (!master) {
> + dev_err(>dev, "master allocation failed\n");
> + return -ENOMEM;
> + }
> +
> + platform_set_drvdata(pdev, master);
> + tqspi = spi_master_get_devdata(master);
> +
> + if (of_property_read_u32(pdev->dev.of_node, "spi-max-frequency",
> +  >max_speed_hz))
> + master->max_speed_hz = QSPI_MAX_SPEED;
> +
> + /* the spi->mode bits understood by this driver: */
> + master->mode_bits = SPI_MODE_0 | SPI_MODE_3 | SPI_CS_HIGH |
> + SPI_TX_DUAL | SPI_RX_DUAL | 

Re: [PATCH v10 17/19] ARM: tegra: Add EMC OPP properties to Tegra20 device-trees

[Jon Hunter]

On 30/11/2020 22:57, Dmitry Osipenko wrote:
> 01.12.2020 00:17, Jon Hunter пишет:
>> Hi Dmitry,
>>
>> On 23/11/2020 00:27, Dmitry Osipenko wrote:
>>> Add EMC OPP DVFS tables and update board device-trees by removing
>>> unsupported OPPs.
>>>
>>> Signed-off-by: Dmitry Osipenko 
>> This change is generating the following warning on Tegra20 Ventana
>> and prevents the EMC from probing ...
>>
>> [2.485711] tegra20-emc 7000f400.memory-controller: device-tree doesn't 
>> have memory timings
>> [2.499386] tegra20-emc 7000f400.memory-controller: 32bit DRAM bus
>> [2.505810] [ cut here ]
>> [2.510511] WARNING: CPU: 0 PID: 1 at 
>> /local/workdir/tegra/mlt-linux_next/kernel/drivers/opp/of.c:875 
>> _of_add_opp_table_v2+0x598/0x61c
>> [2.529746] Modules linked in:
>> [2.540140] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 
>> 5.10.0-rc5-next-20201130 #1
>> [2.554606] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
>> [2.560892] [] (unwind_backtrace) from [] 
>> (show_stack+0x10/0x14)
>> [2.568640] [] (show_stack) from [] 
>> (dump_stack+0xc8/0xdc)
>> [2.575866] [] (dump_stack) from [] 
>> (__warn+0x104/0x108)
>> [2.582912] [] (__warn) from [] 
>> (warn_slowpath_fmt+0xb0/0xb8)
>> [2.590397] [] (warn_slowpath_fmt) from [] 
>> (_of_add_opp_table_v2+0x598/0x61c)
>> [2.599269] [] (_of_add_opp_table_v2) from [] 
>> (dev_pm_opp_of_add_table+0x3c/0x1a0)
>> [2.608582] [] (dev_pm_opp_of_add_table) from [] 
>> (tegra_emc_probe+0x478/0x940)
>> [2.617548] [] (tegra_emc_probe) from [] 
>> (platform_drv_probe+0x48/0x98)
>> [2.625899] [] (platform_drv_probe) from [] 
>> (really_probe+0x218/0x3b8)
>> [2.634162] [] (really_probe) from [] 
>> (driver_probe_device+0x5c/0xb4)
>> [2.642338] [] (driver_probe_device) from [] 
>> (device_driver_attach+0x58/0x60)
>> [2.651208] [] (device_driver_attach) from [] 
>> (__driver_attach+0x80/0xbc)
>> [2.659730] [] (__driver_attach) from [] 
>> (bus_for_each_dev+0x74/0xb4)
>> [2.667905] [] (bus_for_each_dev) from [] 
>> (bus_add_driver+0x164/0x1e8)
>> [2.676168] [] (bus_add_driver) from [] 
>> (driver_register+0x7c/0x114)
>> [2.684259] [] (driver_register) from [] 
>> (do_one_initcall+0x54/0x2b0)
>> [2.692441] [] (do_one_initcall) from [] 
>> (kernel_init_freeable+0x1a4/0x1f4)
>> [2.701145] [] (kernel_init_freeable) from [] 
>> (kernel_init+0x8/0x118)
>> [2.709321] [] (kernel_init) from [] 
>> (ret_from_fork+0x14/0x24)
>> [2.716885] Exception stack(0xc1501fb0 to 0xc1501ff8)
>> [2.721933] 1fa0:   
>>  
>> [2.730106] 1fc0:       
>>  
>> [2.738278] 1fe0:     0013 
>> [2.751940] ---[ end trace 61e3b76deca27ef3 ]---
>>
>>
>> Cheers
>> Jon
>>
> 
> Hello Jon,
> 
> That is harmless and expected to happen because the patch "memory:
> tegra20: Support hardware versioning and clean up OPP table
> initialization" isn't applied yet, while Thierry already applied the DT
> patches from this v10.


Thanks, pulling in that patch did fix it.

Jon

-- 
nvpublic

Re: [PATCH net-next 4/6] net: ipa: add support to code for IPA v4.5

[Jon Hunter]

On 25/11/2020 20:45, Alex Elder wrote:
> Update the IPA code to make use of the updated IPA v4.5 register
> definitions.  Generally what this patch does is, if IPA v4.5
> hardware is in use:
>   - Ensure new registers or fields in IPA v4.5 are updated where
> required
>   - Ensure registers or fields not supported in IPA v4.5 are not
> examined when read, or are set to 0 when written
> It does this while preserving the existing functionality for IPA
> versions lower than v4.5.
> 
> The values to program for QSB_MAX_READS and QSB_MAX_WRITES and the
> source and destination resource counts are updated to be correct for
> all versions through v4.5 as well.
> 
> Note that IPA_RESOURCE_GROUP_SRC_MAX and IPA_RESOURCE_GROUP_DST_MAX
> already reflect that 5 is an acceptable number of resources (which
> IPA v4.5 implements).
> 
> Signed-off-by: Alex Elder 


This change is generating the following build error on ARM64 ...

In file included from drivers/net/ipa/ipa_main.c:9:0:
In function ‘u32_encode_bits’,
inlined from ‘ipa_hardware_config_qsb.isra.7’ at 
drivers/net/ipa/ipa_main.c:286:6,
inlined from ‘ipa_hardware_config’ at drivers/net/ipa/ipa_main.c:363:2,
inlined from ‘ipa_config.isra.12’ at drivers/net/ipa/ipa_main.c:555:2,
inlined from ‘ipa_probe’ at drivers/net/ipa/ipa_main.c:835:6:
./include/linux/bitfield.h:131:3: error: call to ‘__field_overflow’ declared 
with attribute error: value doesn't fit into mask
   __field_overflow(); \
   ^~
./include/linux/bitfield.h:151:2: note: in expansion of macro ‘MAKE_OP’
  MAKE_OP(u##size,u##size,,)
  ^~~
./include/linux/bitfield.h:154:1: note: in expansion of macro ‘__MAKE_OP’
 __MAKE_OP(32)
 ^

Cheers
Jon

-- 
nvpublic

Re: [PATCH v10 17/19] ARM: tegra: Add EMC OPP properties to Tegra20 device-trees

[Jon Hunter]

Hi Dmitry,

On 23/11/2020 00:27, Dmitry Osipenko wrote:
> Add EMC OPP DVFS tables and update board device-trees by removing
> unsupported OPPs.
> 
> Signed-off-by: Dmitry Osipenko 
This change is generating the following warning on Tegra20 Ventana
and prevents the EMC from probing ...

[2.485711] tegra20-emc 7000f400.memory-controller: device-tree doesn't have 
memory timings
[2.499386] tegra20-emc 7000f400.memory-controller: 32bit DRAM bus
[2.505810] [ cut here ]
[2.510511] WARNING: CPU: 0 PID: 1 at 
/local/workdir/tegra/mlt-linux_next/kernel/drivers/opp/of.c:875 
_of_add_opp_table_v2+0x598/0x61c
[2.529746] Modules linked in:
[2.540140] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 
5.10.0-rc5-next-20201130 #1
[2.554606] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
[2.560892] [] (unwind_backtrace) from [] 
(show_stack+0x10/0x14)
[2.568640] [] (show_stack) from [] 
(dump_stack+0xc8/0xdc)
[2.575866] [] (dump_stack) from [] (__warn+0x104/0x108)
[2.582912] [] (__warn) from [] 
(warn_slowpath_fmt+0xb0/0xb8)
[2.590397] [] (warn_slowpath_fmt) from [] 
(_of_add_opp_table_v2+0x598/0x61c)
[2.599269] [] (_of_add_opp_table_v2) from [] 
(dev_pm_opp_of_add_table+0x3c/0x1a0)
[2.608582] [] (dev_pm_opp_of_add_table) from [] 
(tegra_emc_probe+0x478/0x940)
[2.617548] [] (tegra_emc_probe) from [] 
(platform_drv_probe+0x48/0x98)
[2.625899] [] (platform_drv_probe) from [] 
(really_probe+0x218/0x3b8)
[2.634162] [] (really_probe) from [] 
(driver_probe_device+0x5c/0xb4)
[2.642338] [] (driver_probe_device) from [] 
(device_driver_attach+0x58/0x60)
[2.651208] [] (device_driver_attach) from [] 
(__driver_attach+0x80/0xbc)
[2.659730] [] (__driver_attach) from [] 
(bus_for_each_dev+0x74/0xb4)
[2.667905] [] (bus_for_each_dev) from [] 
(bus_add_driver+0x164/0x1e8)
[2.676168] [] (bus_add_driver) from [] 
(driver_register+0x7c/0x114)
[2.684259] [] (driver_register) from [] 
(do_one_initcall+0x54/0x2b0)
[2.692441] [] (do_one_initcall) from [] 
(kernel_init_freeable+0x1a4/0x1f4)
[2.701145] [] (kernel_init_freeable) from [] 
(kernel_init+0x8/0x118)
[2.709321] [] (kernel_init) from [] 
(ret_from_fork+0x14/0x24)
[2.716885] Exception stack(0xc1501fb0 to 0xc1501ff8)
[2.721933] 1fa0:   
 
[2.730106] 1fc0:       
 
[2.738278] 1fe0:     0013 
[2.751940] ---[ end trace 61e3b76deca27ef3 ]---


Cheers
Jon

-- 
nvpublic

Re: [PATCH] dmaengine: tegra-apb: fix reference leak in tegra_dma_issue_pending and tegra_dma_synchronize

[Jon Hunter]

On 27/11/2020 09:44, Qinglang Miao wrote:
> pm_runtime_get_sync will increment pm usage counter even it
> failed. Forgetting to putting operation will result in a
> reference leak here.
> 
> A new function pm_runtime_resume_and_get is introduced in
> [0] to keep usage counter balanced. So We fix the reference
> leak by replacing it with new funtion.
> 
> [0] dd8088d5a896 ("PM: runtime: Add  pm_runtime_resume_and_get to deal with 
> usage counter")
> 
> Fixes: 84a3f375eea9 ("dmaengine: tegra-apb: Keep clock enabled only during of 
> DMA transfer")
> Fixes: 664475cffb8c ("dmaengine: tegra-apb: Ensure that clock is enabled 
> during of DMA synchronization")
> Reported-by: Hulk Robot 
> Signed-off-by: Qinglang Miao 
> ---
>  drivers/dma/tegra20-apb-dma.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
> index 71827d9b0..b7260749e 100644
> --- a/drivers/dma/tegra20-apb-dma.c
> +++ b/drivers/dma/tegra20-apb-dma.c
> @@ -723,7 +723,7 @@ static void tegra_dma_issue_pending(struct dma_chan *dc)
>   goto end;
>   }
>   if (!tdc->busy) {
> - err = pm_runtime_get_sync(tdc->tdma->dev);
> + err = pm_runtime_resume_and_get(tdc->tdma->dev);
>   if (err < 0) {
>   dev_err(tdc2dev(tdc), "Failed to enable DMA\n");
>   goto end;
> @@ -818,7 +818,7 @@ static void tegra_dma_synchronize(struct dma_chan *dc)
>   struct tegra_dma_channel *tdc = to_tegra_dma_chan(dc);
>   int err;
>  
> - err = pm_runtime_get_sync(tdc->tdma->dev);
> + err = pm_runtime_resume_and_get(tdc->tdma->dev);
>   if (err < 0) {
>   dev_err(tdc2dev(tdc), "Failed to synchronize DMA: %d\n", err);
>   return;


Reviewed-by: Jon Hunter 

Cheers
Jon

-- 
nvpublic

Re: [PATCH v3 1/6] regulator: core: validate selector against linear_min_sel

[Jon Hunter]

On 24/11/2020 11:14, claudiu.bez...@microchip.com wrote:

...

> I would say that a solution would be to have a new helper to retrieve the
> linear_min_sel (e.g. regulator_min_sel()) and use this for all the
> consumers of regulator_list_voltage() and the other APIs that patch
> "regulator: core: validate selector against linear_min_sel" has changed
> (regulator_list_voltage_table(), regulator_set_voltage_time()). With this
> change the loop in find_vdd_map_entry_exact() should be b/w
> regulator_min_sel() and regulator_count_voltages().
> 
> Maybe Mark has a better solution for this.


By the way, I don't think that Tegra is alone here. I see some other
drivers doing some similar things [0][1][2] and so I am wondering if
this is going to be a problem for a few drivers.

Jon

[0]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/mmc/core/regulator.c#n61
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/cpufreq/s3c2416-cpufreq.c#n263
[2]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/leds/leds-regulator.c#n29

-- 
nvpublic

[PATCH] dt-bindings: Correct GV11B GPU register sizes

[Jon Hunter]

Commit 90a09178f309 ("dt-bindings: Add documentation for GV11B GPU")
added the GV11B GPU device-tree bindings information but incorrectly
added an additional 0 to the size of the addresses in the example.

Fixes: 90a09178f309 ("dt-bindings: Add documentation for GV11B GPU")
Signed-off-by: Jon Hunter 
---
 Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt 
b/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt
index 662a3c8a7d29..cc6ce5221a38 100644
--- a/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt
+++ b/Documentation/devicetree/bindings/gpu/nvidia,gk20a.txt
@@ -97,8 +97,8 @@ Example for GV11B:
 
gpu@1700 {
compatible = "nvidia,gv11b";
-   reg = <0x1700 0x1000>,
- <0x1800 0x1000>;
+   reg = <0x1700 0x100>,
+ <0x1800 0x100>;
interrupts = ,
 ;
interrupt-names = "stall", "nonstall";
-- 
2.25.1

Re: [PATCH v3 1/6] regulator: core: validate selector against linear_min_sel

[Jon Hunter]

On 13/11/2020 15:21, Claudiu Beznea wrote:
> There are regulators who's min selector is not zero. Selectors loops
> (looping b/w zero and regulator::desc::n_voltages) might throw errors
> because invalid selectors are used (lower than
> regulator::desc::linear_min_sel). For this situations validate selectors
> against regulator::desc::linear_min_sel.


After this commit was merged, I noticed a regression in the DFLL (CPU
clock source) on Tegra124. The DFLL driver
(drivers/clk/tegra/clk-dfll.c) calls regulator_list_voltage() in a loop
to determine the selector for a given voltage (see function
find_vdd_map_entry_exact()).

Currently, the DFLL driver queries the number of voltages provided by
the regulator by calling regulator_count_voltages() and then starting
from 0, iterates through the number of voltages to find the selector
value for the voltage it is looking for by calling
regulator_list_voltage(). It assumes that any negative value returned by
calling regulator_list_voltage() is an error and this will cause the
loop up to terminate.

In this case the regulator in question is the as3722 and the
linear_min_sel for this regulator is 1 and so when the DFLL driver calls
regulator_list_voltage() with a selector value of 0 it now returns a
negative error code, as expected by this change, and this terminates the
loop up in the DFLL driver. So I can clearly see why this is happening
and I could fix up the DFLL driver to avoid this.

Before doing so, I wanted to ask if that is the correct fix here,
because it seems a bit odd that regulator_count_voltages() returns N
voltages, but if the min selector value is greater than 0, then actually
there are less than N. However, changing the number of voltages
supported by the regulator to be N - linear_min_sel does not make sense
either because then we need to know the linear_min_sel in order to
determine the first valid voltage.

In case of the as3722, the value 0 means that the regulator is powered
down. So it is a valid setting and equates to 0 volts at the output AFAICT.

Please let me know your thoughts are the correct way to fix this up.

Thanks
Jon

-- 
nvpublic

Re: [PATCH v5 0/6] Tegra210 audio graph card

[Jon Hunter]

On 11/11/2020 18:34, Sameer Pujar wrote:
> This series adds audio graph based sound card support for Tegra210
> platforms like Jetson-TX1 an Jetson-Nano. The following preparatory
> audio graph enhancement series is already merged.
>  * https://patchwork.kernel.org/project/alsa-devel/list/?series=375629=*
> 
> Following are the summary of changes:
>  * Add graph/audio-graph based schemas or schema updates for Tegra210
>component and machine drivers.
>  * Add Tegra audio graph machine driver.
>  * Add required DT support for Jetson-TX1/Nano.
> 
> This work is based on earlier discussion of DPCM usage for Tegra
> and simple card driver updates.
>  * https://lkml.org/lkml/2020/4/30/519
>  * https://lkml.org/lkml/2020/6/27/4
> 
> This series has dependency over following graph and audio-graph series.
>  * 
> http://patchwork.ozlabs.org/project/devicetree-bindings/patch/20201102203656.220187-2-r...@kernel.org/
>  * https://patchwork.kernel.org/project/alsa-devel/list/?series=382009=*


Only one minor comment, but this looks good to me. Otherwise for the
series ...

Reviewed-by: Jon Hunter 

Cheers
Jon

-- 
nvpublic

Re: [PATCH v5 3/6] ASoC: tegra: Add audio graph based card driver

[Jon Hunter]

On 11/11/2020 18:34, Sameer Pujar wrote:
> Add Tegra audio machine driver which is based on generic audio graph card
> driver. It re-uses most of the common stuff from audio graph driver and
> uses the same DT binding. Required Tegra specific customizations are done
> in the driver and additional DT bindings are required for clock handling.
> 
> Details on the customizations done:
> 
>  - Update PLL rates at runtime: Tegra HW supports multiple sample rates
>(multiples of 8x and 11.025x) and both of these groups require different
>PLL rates. Hence there is a requirement to update this at runtime.
>This is achieved by providing a custom 'snd_soc_ops' and in hw_param()
>callback PLL rate is updated as per the sample rate.
> 
>  - Internal structure 'tegra_audio_graph_data' is used to maintain clock
>handles of PLL.
> 
>  - The 'force_dpcm' flag is set to use DPCM for all DAI links.
> 
>  - The 'component_chaining' flag is set to use DPCM with component model.
> 
> Signed-off-by: Sameer Pujar 
> ---
>  sound/soc/tegra/Kconfig  |   9 ++
>  sound/soc/tegra/Makefile |   2 +
>  sound/soc/tegra/tegra_audio_graph_card.c | 255 
> +++
>  3 files changed, 266 insertions(+)
>  create mode 100644 sound/soc/tegra/tegra_audio_graph_card.c
> 
> diff --git a/sound/soc/tegra/Kconfig b/sound/soc/tegra/Kconfig
> index a62cc87..6dc83ad 100644
> --- a/sound/soc/tegra/Kconfig
> +++ b/sound/soc/tegra/Kconfig
> @@ -117,6 +117,15 @@ config SND_SOC_TEGRA210_ADMAIF
> channel. Buffer size is configurable for each ADMAIIF channel.
> Say Y or M if you want to add support for Tegra210 ADMAIF module.
>  
> +config SND_SOC_TEGRA_AUDIO_GRAPH_CARD
> + tristate "Audio Graph Card based Tegra driver"
> + depends on SND_AUDIO_GRAPH_CARD
> + help
> +   Config to enable Tegra audio machine driver based on generic
> +   audio graph driver. It is a thin driver written to customize
> +   few things for Tegra audio. Most of the code is re-used from
> +   audio graph driver and the same DT bindings are used.
> +
>  config SND_SOC_TEGRA_RT5640
>   tristate "SoC Audio support for Tegra boards using an RT5640 codec"
>   depends on SND_SOC_TEGRA && I2C && GPIOLIB
> diff --git a/sound/soc/tegra/Makefile b/sound/soc/tegra/Makefile
> index 60040a0..b17dd6e 100644
> --- a/sound/soc/tegra/Makefile
> +++ b/sound/soc/tegra/Makefile
> @@ -38,6 +38,7 @@ snd-soc-tegra-trimslice-objs := trimslice.o
>  snd-soc-tegra-alc5632-objs := tegra_alc5632.o
>  snd-soc-tegra-max98090-objs := tegra_max98090.o
>  snd-soc-tegra-sgtl5000-objs := tegra_sgtl5000.o
> +snd-soc-tegra-audio-graph-card-objs := tegra_audio_graph_card.o
>  
>  obj-$(CONFIG_SND_SOC_TEGRA_RT5640) += snd-soc-tegra-rt5640.o
>  obj-$(CONFIG_SND_SOC_TEGRA_RT5677) += snd-soc-tegra-rt5677.o
> @@ -48,3 +49,4 @@ obj-$(CONFIG_SND_SOC_TEGRA_TRIMSLICE) += 
> snd-soc-tegra-trimslice.o
>  obj-$(CONFIG_SND_SOC_TEGRA_ALC5632) += snd-soc-tegra-alc5632.o
>  obj-$(CONFIG_SND_SOC_TEGRA_MAX98090) += snd-soc-tegra-max98090.o
>  obj-$(CONFIG_SND_SOC_TEGRA_SGTL5000) += snd-soc-tegra-sgtl5000.o
> +obj-$(CONFIG_SND_SOC_TEGRA_AUDIO_GRAPH_CARD) += 
> snd-soc-tegra-audio-graph-card.o
> diff --git a/sound/soc/tegra/tegra_audio_graph_card.c 
> b/sound/soc/tegra/tegra_audio_graph_card.c
> new file mode 100644
> index 000..f4d826d
> --- /dev/null
> +++ b/sound/soc/tegra/tegra_audio_graph_card.c
> @@ -0,0 +1,255 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +//
> +// tegra_audio_graph_card.c - Audio Graph based Tegra Machine Driver
> +//
> +// Copyright (c) 2020 NVIDIA CORPORATION.  All rights reserved.
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define MAX_PLLA_OUT0_DIV 128
> +
> +#define simple_to_tegra_priv(simple) \
> + container_of(simple, struct tegra_audio_priv, simple)
> +
> +enum srate_type {
> + /*
> +  * Sample rates multiple of 8000 Hz and below are supported:
> +  * ( 8000, 16000, 32000, 48000, 96000, 192000 Hz )
> +  */
> + x8_RATE,
> +
> + /*
> +  * Sample rates multiple of 11025 Hz and below are supported:
> +  * ( 11025, 22050, 44100, 88200, 176400 Hz )
> +  */
> + x11_RATE,
> +
> + NUM_RATE_TYPE,
> +};
> +
> +struct tegra_audio_priv {
> + struct asoc_simple_priv simple;
> + struct clk *clk_plla_out0;
> + struct clk *clk_plla;
> +};
> +
> +/* Tegra audio chip data */
> +struct tegra_audio_cdata {
> + unsigned int plla_rates[NUM_RATE_TYPE];
> + unsigned int plla_out0_rates[NUM_RATE_TYPE];
> +};
> +
> +/* Setup PLL clock as per the given sample rate */
> +static int tegra_audio_graph_update_pll(struct snd_pcm_substream *substream,
> + struct snd_pcm_hw_params *params)
> +{
> + struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
> + struct asoc_simple_priv *simple = snd_soc_card_get_drvdata(rtd->card);
> + 

Re: [PATCH v1] arm64: tegra: jetson-tx1: Fix USB_VBUS_EN0 regulator

[Jon Hunter]

On 18/11/2020 03:46, JC Kuo wrote:
> USB_VBUS_EN0 regulator (regulator@11) is being overwritten by vdd-cam-1v2
> regulator. This commit rearrange USB_VBUS_EN0 to be regulator@14.
> 
> Signed-off-by: JC Kuo 
> ---
>  .../arm64/boot/dts/nvidia/tegra210-p2597.dtsi | 20 +--
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi 
> b/arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi
> index e18e1a9a3011..a9caaf7c0d67 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra210-p2597.dtsi
> @@ -1663,16 +1663,6 @@ vdd_usb_vbus: regulator@9 {
>   vin-supply = <_5v0_sys>;
>   };
>  
> - vdd_usb_vbus_otg: regulator@11 {
> - compatible = "regulator-fixed";
> - regulator-name = "USB_VBUS_EN0";
> - regulator-min-microvolt = <500>;
> - regulator-max-microvolt = <500>;
> - gpio = < TEGRA_GPIO(CC, 4) GPIO_ACTIVE_HIGH>;
> - enable-active-high;
> - vin-supply = <_5v0_sys>;
> - };
> -
>   vdd_hdmi: regulator@10 {
>   compatible = "regulator-fixed";
>   regulator-name = "VDD_HDMI_5V0";
> @@ -1712,4 +1702,14 @@ vdd_cam_1v8: regulator@13 {
>   enable-active-high;
>   vin-supply = <_3v3_sys>;
>   };
> +
> + vdd_usb_vbus_otg: regulator@14 {
> + compatible = "regulator-fixed";
> + regulator-name = "USB_VBUS_EN0";
> + regulator-min-microvolt = <500>;
> + regulator-max-microvolt = <500>;
> + gpio = < TEGRA_GPIO(CC, 4) GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> + vin-supply = <_5v0_sys>;
> + };
>  };
> 

Thanks for catching this! We should add the 'Fixes:' tag.

By the way, I assume that VBUS is currently broken for the OTG port.
Without this change is that USB port completely broken? I am wondering
if we need to CC sta...@vger.kernel.org on this.

Reviewed-by: Jon Hunter 

Cheers
Jon

-- 
nvpublic

Re: [PATCH] cpufreq: tegra186: Fix get frequency callback

[Jon Hunter]

Hi Rafael,

On 04/11/2020 09:33, Viresh Kumar wrote:
> On 03-11-20, 11:55, Jon Hunter wrote:
>> Commit b89c01c96051 ("cpufreq: tegra186: Fix initial frequency")
>> implemented the CPUFREQ 'get' callback to determine the current
>> operating frequency for each CPU. This implementation used a simple
>> looked up to determine the current operating frequency. The problem
>> with this is that frequency table for different Tegra186 devices may
>> vary and so the default boot frequency for Tegra186 device may or may
>> not be present in the frequency table. If the default boot frequency is
>> not present in the frequency table, this causes the function
>> tegra186_cpufreq_get() to return 0 and in turn causes cpufreq_online()
>> to fail which prevents CPUFREQ from working.
>>
>> Fix this by always calculating the CPU frequency based upon the current
>> 'ndiv' setting for the CPU. Note that the CPU frequency for Tegra186 is
>> calculated by reading the current 'ndiv' setting, multiplying by the
>> CPU reference clock and dividing by a constant divisor.
>>
>> Fixes: b89c01c96051 ("cpufreq: tegra186: Fix initial frequency")
>>
>> Signed-off-by: Jon Hunter 
>> ---
>>  drivers/cpufreq/tegra186-cpufreq.c | 33 +++---
>>  1 file changed, 21 insertions(+), 12 deletions(-)
> 
> Acked-by: Viresh Kumar 
> 
> Rafael: This needs to go in the next rc and so I am not applying it
> in my tree as this is the only fix I have for now.


Are you able to pick this up for v5.10 fixes?

Thanks
Jon

-- 
nvpublic

Re: [PATCH] ARM: tegra: Populate OPP table for Tegra20 Ventana

[Jon Hunter]

On 12/11/2020 12:11, Dmitry Osipenko wrote:
> 11.11.2020 13:38, Jon Hunter пишет:
>> Commit 9ce274630495 ("cpufreq: tegra20: Use generic cpufreq-dt driver
>> (Tegra30 supported now)") update the Tegra20 CPUFREQ driver to use the
>> generic CPUFREQ device-tree driver. Since this change CPUFREQ support
>> on the Tegra20 Ventana platform has been broken because the necessary
>> device-tree nodes with the operating point information are not populated
>> for this platform. Fix this by updating device-tree for Venata to
>> include the operating point informration for Tegra20.
>>
>> Fixes: 9ce274630495 ("cpufreq: tegra20: Use generic cpufreq-dt driver 
>> (Tegra30 supported now)")
>> Cc: sta...@vger.kernel.org
>>
>> Signed-off-by: Jon Hunter 
>> ---
>>  arch/arm/boot/dts/tegra20-ventana.dts | 11 +++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/tegra20-ventana.dts 
>> b/arch/arm/boot/dts/tegra20-ventana.dts
>> index b158771ac0b7..055334ae3d28 100644
>> --- a/arch/arm/boot/dts/tegra20-ventana.dts
>> +++ b/arch/arm/boot/dts/tegra20-ventana.dts
>> @@ -3,6 +3,7 @@
>>  
>>  #include 
>>  #include "tegra20.dtsi"
>> +#include "tegra20-cpu-opp.dtsi"
>>  
>>  / {
>>  model = "NVIDIA Tegra20 Ventana evaluation board";
>> @@ -592,6 +593,16 @@ clk32k_in: clock@0 {
>>  #clock-cells = <0>;
>>  };
>>  
>> +cpus {
>> +cpu0: cpu@0 {
> 
> I assume you're going to use this cpu0 handle later on.

Opps, I will remove that.

> 
>> +operating-points-v2 = <_opp_table>;
>> +};
>> +
>> +cpu@1 {
>> +operating-points-v2 = <_opp_table>;
>> +};
>> +};
>> +
>>  gpio-keys {
>>  compatible = "gpio-keys";
>>  
>>
> 
> Reviewed-by: Dmitry Osipenko 
> 

Thanks!
Jon

-- 
nvpublic

[PATCH V2] ARM: tegra: Populate OPP table for Tegra20 Ventana

[Jon Hunter]

Commit 9ce274630495 ("cpufreq: tegra20: Use generic cpufreq-dt driver
(Tegra30 supported now)") update the Tegra20 CPUFREQ driver to use the
generic CPUFREQ device-tree driver. Since this change CPUFREQ support
on the Tegra20 Ventana platform has been broken because the necessary
device-tree nodes with the operating point information are not populated
for this platform. Fix this by updating device-tree for Venata to
include the operating point informration for Tegra20.

Fixes: 9ce274630495 ("cpufreq: tegra20: Use generic cpufreq-dt driver (Tegra30 
supported now)")
Cc: sta...@vger.kernel.org

Signed-off-by: Jon Hunter 
---
Changes since V1:
- Remove unneeded 'cpu0' phandle

 arch/arm/boot/dts/tegra20-ventana.dts | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/boot/dts/tegra20-ventana.dts 
b/arch/arm/boot/dts/tegra20-ventana.dts
index b158771ac0b7..1b2a0dcd929a 100644
--- a/arch/arm/boot/dts/tegra20-ventana.dts
+++ b/arch/arm/boot/dts/tegra20-ventana.dts
@@ -3,6 +3,7 @@
 
 #include 
 #include "tegra20.dtsi"
+#include "tegra20-cpu-opp.dtsi"
 
 / {
model = "NVIDIA Tegra20 Ventana evaluation board";
@@ -592,6 +593,16 @@ clk32k_in: clock@0 {
#clock-cells = <0>;
};
 
+   cpus {
+   cpu@0 {
+   operating-points-v2 = <_opp_table>;
+   };
+
+   cpu@1 {
+   operating-points-v2 = <_opp_table>;
+   };
+   };
+
gpio-keys {
compatible = "gpio-keys";
 
-- 
2.25.1

Re: [PATCH] ARM: tegra: Populate OPP table for Tegra20 Ventana

[Jon Hunter]

On 12/11/2020 10:51, Dmitry Osipenko wrote:

...

> If you don't see a message in KMSG saying "bringing vdd_cpu to
> 100uV", then should be good.

The bootlog shows ...

[2.271768] tps6586x 3-0034: Found TPS658621C/D, VERSIONCRC is 2c

[2.280320] vdd_sys: supplied by vdd_5v0

[2.285153] vdd_sm0,vdd_core: supplied by vdd_sys

[2.294231] vdd_sm1,vdd_cpu: supplied by vdd_sys

[2.299984] vdd_sm2,vin_ldo*: supplied by vdd_sys

[2.305285] REG-LDO_0: supplied by vdd_sm2,vin_ldo*

[2.311492] vdd_ldo1,avdd_pll*: supplied by vdd_sm2,vin_ldo*

[2.318053] vdd_ldo2,vdd_rtc: supplied by vdd_sm2,vin_ldo*

[2.324786] vdd_ldo3,avdd_usb*: supplied by vdd_sm2,vin_ldo*

[2.331848] vdd_ldo4,avdd_osc,vddio_sys: supplied by vdd_sm2,vin_ldo*

[2.339104] vdd_ldo5,vcore_mmc: supplied by vdd_sys

[2.37] vdd_ldo6,avdd_vdac: Bringing 285uV into 180-180uV

[2.352226] vdd_ldo6,avdd_vdac: supplied by vdd_sm2,vin_ldo*

[2.358814] vdd_ldo7,avdd_hdmi,vdd_fuse: supplied by vdd_sm2,vin_ldo*

[2.366176] vdd_ldo8,avdd_hdmi_pll: supplied by vdd_sm2,vin_ldo*

[2.372959] vdd_ldo9,avdd_2v85,vdd_ddr_rx: supplied by vdd_sm2,vin_ldo*

[2.380373] vdd_rtc_out,vdd_cell: supplied by vdd_sys


Jon

Re: [PATCH] ARM: tegra: Populate OPP table for Tegra20 Ventana

[Jon Hunter]

On 11/11/2020 13:47, Dmitry Osipenko wrote:
> 11.11.2020 13:38, Jon Hunter пишет:
>> Commit 9ce274630495 ("cpufreq: tegra20: Use generic cpufreq-dt driver
>> (Tegra30 supported now)") update the Tegra20 CPUFREQ driver to use the
>> generic CPUFREQ device-tree driver. Since this change CPUFREQ support
>> on the Tegra20 Ventana platform has been broken because the necessary
>> device-tree nodes with the operating point information are not populated
>> for this platform. Fix this by updating device-tree for Venata to
>> include the operating point informration for Tegra20.
>>
>> Fixes: 9ce274630495 ("cpufreq: tegra20: Use generic cpufreq-dt driver 
>> (Tegra30 supported now)")
>> Cc: sta...@vger.kernel.org
>>
>> Signed-off-by: Jon Hunter 
>> ---
>>  arch/arm/boot/dts/tegra20-ventana.dts | 11 +++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/tegra20-ventana.dts 
>> b/arch/arm/boot/dts/tegra20-ventana.dts
>> index b158771ac0b7..055334ae3d28 100644
>> --- a/arch/arm/boot/dts/tegra20-ventana.dts
>> +++ b/arch/arm/boot/dts/tegra20-ventana.dts
>> @@ -3,6 +3,7 @@
>>  
>>  #include 
>>  #include "tegra20.dtsi"
>> +#include "tegra20-cpu-opp.dtsi"
>>  
>>  / {
>>  model = "NVIDIA Tegra20 Ventana evaluation board";
>> @@ -592,6 +593,16 @@ clk32k_in: clock@0 {
>>  #clock-cells = <0>;
>>  };
>>  
>> +cpus {
>> +cpu0: cpu@0 {
>> +operating-points-v2 = <_opp_table>;
>> +};
>> +
>> +cpu@1 {
>> +operating-points-v2 = <_opp_table>;
>> +};
>> +};
>> +
>>  gpio-keys {
>>  compatible = "gpio-keys";
>>  
>>
> 
> This could be wrong to do because CPU voltage is fixed to 1000mV in
> Ventana's DT, are you sure that higher clock rates don't require higher
> voltages? What is the CPU process ID and SoC speedo ID on Ventana?

I see this in the bootlog ...

[2.797684] tegra20-cpufreq tegra20-cpufreq: hardware version 0x2 0x2

> You could easily hook up CPU voltage scaling, please see acer-500 DT and
> patch [1] for examples of how to set up regulators in DT. But then it
> shouldn't be a stable patch.

According to the Ventana design guide the CPU voltage range is 0.8-1.0V
and so it appears to be set to the max. The CPUFREQ test is reporting
the following ...

cpu: cpufreq: - CPU#0:
cpu: cpufreq:   - supported governors:
cpu: cpufreq: - ondemand *
cpu: cpufreq: - performance
cpu: cpufreq: - schedutil
cpu: cpufreq:   - supported rates:
cpu: cpufreq: -  216000
cpu: cpufreq: -  312000
cpu: cpufreq: -  456000
cpu: cpufreq: -  608000
cpu: cpufreq: -  76
cpu: cpufreq: -  816000
cpu: cpufreq: -  912000
cpu: cpufreq: - 100 *
cpu: cpufreq: - CPU#1:
cpu: cpufreq:   - supported governors:
cpu: cpufreq: - ondemand *
cpu: cpufreq: - performance
cpu: cpufreq: - schedutil
cpu: cpufreq:   - supported rates:
cpu: cpufreq: -  216000
cpu: cpufreq: -  312000
cpu: cpufreq: -  456000
cpu: cpufreq: -  608000
cpu: cpufreq: -  76
cpu: cpufreq: -  816000
cpu: cpufreq: -  912000
cpu: cpufreq: - 100 *

Cheers
Jon

-- 
nvpublic

Re: [PATCH] ARM: tegra: Populate OPP table for Tegra20 Ventana

[Jon Hunter]

On 11/11/2020 13:47, Dmitry Osipenko wrote:
> 11.11.2020 13:38, Jon Hunter пишет:
>> Commit 9ce274630495 ("cpufreq: tegra20: Use generic cpufreq-dt driver
>> (Tegra30 supported now)") update the Tegra20 CPUFREQ driver to use the
>> generic CPUFREQ device-tree driver. Since this change CPUFREQ support
>> on the Tegra20 Ventana platform has been broken because the necessary
>> device-tree nodes with the operating point information are not populated
>> for this platform. Fix this by updating device-tree for Venata to
>> include the operating point informration for Tegra20.
>>
>> Fixes: 9ce274630495 ("cpufreq: tegra20: Use generic cpufreq-dt driver 
>> (Tegra30 supported now)")
>> Cc: sta...@vger.kernel.org
>>
>> Signed-off-by: Jon Hunter 
>> ---
>>  arch/arm/boot/dts/tegra20-ventana.dts | 11 +++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/tegra20-ventana.dts 
>> b/arch/arm/boot/dts/tegra20-ventana.dts
>> index b158771ac0b7..055334ae3d28 100644
>> --- a/arch/arm/boot/dts/tegra20-ventana.dts
>> +++ b/arch/arm/boot/dts/tegra20-ventana.dts
>> @@ -3,6 +3,7 @@
>>  
>>  #include 
>>  #include "tegra20.dtsi"
>> +#include "tegra20-cpu-opp.dtsi"
>>  
>>  / {
>>  model = "NVIDIA Tegra20 Ventana evaluation board";
>> @@ -592,6 +593,16 @@ clk32k_in: clock@0 {
>>  #clock-cells = <0>;
>>  };
>>  
>> +cpus {
>> +cpu0: cpu@0 {
>> +operating-points-v2 = <_opp_table>;
>> +};
>> +
>> +cpu@1 {
>> +operating-points-v2 = <_opp_table>;
>> +};
>> +};
>> +
>>  gpio-keys {
>>  compatible = "gpio-keys";
>>  
>>
> 
> This could be wrong to do because CPU voltage is fixed to 1000mV in
> Ventana's DT, are you sure that higher clock rates don't require higher
> voltages? What is the CPU process ID and SoC speedo ID on Ventana?

I looked at that and I did not see any voltage scaling being done with
the previous version of the CPUFREQ driver on Ventana. I will double
check but if it should be then I guess that was broken before.

> You could easily hook up CPU voltage scaling, please see acer-500 DT and
> patch [1] for examples of how to set up regulators in DT. But then it
> shouldn't be a stable patch.

That assumes that you are in the same country as the board you are
testing on :-)

I went back and verified that CPUFREQ scaling is working on stable
kernels 4.4, 4.9, 4.14, 4.19 and 5.4 on Ventana. It is only after your
change that it no longer works and yes it should be in stable.

Jon

-- 
nvpublic

[PATCH] ARM64: tegra: Correct the UART for Jetson Xavier NX

[Jon Hunter]

The Jetson Xavier NX board routes UARTA to the 40-pin header and UARTC
to a 12-pin debug header. The UARTs can be used by either the Tegra
Combined UART (TCU) driver or the Tegra 8250 driver. By default, the
TCU will use UARTC on Jetson Xavier NX. Currently, device-tree for
Xavier NX enables the TCU and the Tegra 8250 node for UARTC. Fix this
by disabling the Tegra 8250 node for UARTC and enabling the Tegra 8250
node for UARTA.

Fixes: 3f9efbbe57bc ("arm64: tegra: Add support for Jetson Xavier NX")
Cc: sta...@vger.kernel.org

Signed-off-by: Jon Hunter 
---
 arch/arm64/boot/dts/nvidia/tegra194-p3668-.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/nvidia/tegra194-p3668-.dtsi 
b/arch/arm64/boot/dts/nvidia/tegra194-p3668-.dtsi
index a2893be80507..0dc8304a2edd 100644
--- a/arch/arm64/boot/dts/nvidia/tegra194-p3668-.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra194-p3668-.dtsi
@@ -54,7 +54,7 @@ memory-controller@2c0 {
status = "okay";
};
 
-   serial@c28 {
+   serial@310 {
status = "okay";
};
 
-- 
2.25.1

[PATCH] ARM: tegra: Populate OPP table for Tegra20 Ventana

[Jon Hunter]

Commit 9ce274630495 ("cpufreq: tegra20: Use generic cpufreq-dt driver
(Tegra30 supported now)") update the Tegra20 CPUFREQ driver to use the
generic CPUFREQ device-tree driver. Since this change CPUFREQ support
on the Tegra20 Ventana platform has been broken because the necessary
device-tree nodes with the operating point information are not populated
for this platform. Fix this by updating device-tree for Venata to
include the operating point informration for Tegra20.

Fixes: 9ce274630495 ("cpufreq: tegra20: Use generic cpufreq-dt driver (Tegra30 
supported now)")
Cc: sta...@vger.kernel.org

Signed-off-by: Jon Hunter 
---
 arch/arm/boot/dts/tegra20-ventana.dts | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/boot/dts/tegra20-ventana.dts 
b/arch/arm/boot/dts/tegra20-ventana.dts
index b158771ac0b7..055334ae3d28 100644
--- a/arch/arm/boot/dts/tegra20-ventana.dts
+++ b/arch/arm/boot/dts/tegra20-ventana.dts
@@ -3,6 +3,7 @@
 
 #include 
 #include "tegra20.dtsi"
+#include "tegra20-cpu-opp.dtsi"
 
 / {
model = "NVIDIA Tegra20 Ventana evaluation board";
@@ -592,6 +593,16 @@ clk32k_in: clock@0 {
#clock-cells = <0>;
};
 
+   cpus {
+   cpu0: cpu@0 {
+   operating-points-v2 = <_opp_table>;
+   };
+
+   cpu@1 {
+   operating-points-v2 = <_opp_table>;
+   };
+   };
+
gpio-keys {
compatible = "gpio-keys";
 
-- 
2.25.1

[PATCH] phy: tegra: Don't warn on probe deferral

[Jon Hunter]

Deferred probe is an expected return value for devm_regulator_bulk_get().
Given that the driver deals with it properly, there's no need to output a
warning that may potentially confuse users.

Signed-off-by: Jon Hunter 
---
 drivers/phy/tegra/xusb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
index ad88d74c1884..2eafb813825b 100644
--- a/drivers/phy/tegra/xusb.c
+++ b/drivers/phy/tegra/xusb.c
@@ -1200,7 +1200,7 @@ static int tegra_xusb_padctl_probe(struct platform_device 
*pdev)
err = devm_regulator_bulk_get(>dev, padctl->soc->num_supplies,
  padctl->supplies);
if (err < 0) {
-   dev_err(>dev, "failed to get regulators: %d\n", err);
+   dev_err_probe(>dev, err, "failed to get regulators\n");
goto remove;
}
 
-- 
2.25.1

[PATCH V3] drm/tegra: sor: Don't warn on probe deferral

[Jon Hunter]

Deferred probe is an expected return value for tegra_output_probe().
Given that the driver deals with it properly, there's no need to output
a warning that may potentially confuse users.

Signed-off-by: Jon Hunter 
---
Changes since V2:
- Removed duplicate errno print
Changes since V1:
- This time, I actually validated it!

 drivers/gpu/drm/tegra/sor.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
index e88a17c2937f..a5b944dacb35 100644
--- a/drivers/gpu/drm/tegra/sor.c
+++ b/drivers/gpu/drm/tegra/sor.c
@@ -3764,10 +3764,9 @@ static int tegra_sor_probe(struct platform_device *pdev)
return err;
 
err = tegra_output_probe(>output);
-   if (err < 0) {
-   dev_err(>dev, "failed to probe output: %d\n", err);
-   return err;
-   }
+   if (err < 0)
+   return dev_err_probe(>dev, err,
+"failed to probe output\n");
 
if (sor->ops && sor->ops->probe) {
err = sor->ops->probe(sor);
-- 
2.25.1

Re: [PATCH V2] drm/tegra: sor: Don't warn on probe deferral

[Jon Hunter]

On 04/11/2020 10:49, Dmitry Osipenko wrote:
> 04.11.2020 12:23, Jon Hunter пишет:
>> Deferred probe is an expected return value for tegra_output_probe().
>> Given that the driver deals with it properly, there's no need to output
>> a warning that may potentially confuse users.
>>
>> Signed-off-by: Jon Hunter 
>> ---
>>
>> Changes since V1:
>> - This time, I actually validated it!
>>
>>  drivers/gpu/drm/tegra/sor.c | 7 +++
>>  1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
>> index e88a17c2937f..898a80ca37fa 100644
>> --- a/drivers/gpu/drm/tegra/sor.c
>> +++ b/drivers/gpu/drm/tegra/sor.c
>> @@ -3764,10 +3764,9 @@ static int tegra_sor_probe(struct platform_device 
>> *pdev)
>>  return err;
>>  
>>  err = tegra_output_probe(>output);
>> -if (err < 0) {
>> -dev_err(>dev, "failed to probe output: %d\n", err);
>> -return err;
>> -}
>> +if (err < 0)
>> +return dev_err_probe(>dev, err,
>> + "failed to probe output: %d\n", err);
> 
> Hello Jon,
> 
> There is no need to duplicate the error code in the message [1]. Perhaps
> worth making a v3? :)
Indeed! Thanks for catching. Trying to do to many things at the same
time. I should have learned by now!

Jon

-- 
nvpublic

Re: [PATCH 0/3] Add support to handle prefetchable memory

[Jon Hunter]

On 26/10/2020 12:32, Thierry Reding wrote:
> On Sat, Oct 24, 2020 at 04:03:41AM +, Jingoo Han wrote:
>> On 10/23/20, 3:57 PM, Vidya Sagar wrote:
>>>
>>> This patch series adds support for configuring the DesignWare IP's ATU
>>> region for prefetchable memory translations.
>>> It first starts by flagging a warning if the size of non-prefetchable
>>> aperture goes beyond 32-bit as PCIe spec doesn't allow it.
>>> And then adds required support for programming the ATU to handle higher
>>> (i.e. >4GB) sizes and then finally adds support for differentiating
>>> between prefetchable and non-prefetchable regions and configuring one of
>>> the ATU regions for prefetchable memory translations purpose.
>>>
>>> Vidya Sagar (3):
>>>   PCI: of: Warn if non-prefetchable memory aperture size is > 32-bit
>>>   PCI: dwc: Add support to program ATU for >4GB memory aperture sizes
>>>   PCI: dwc: Add support to handle prefetchable memory mapping
>>
>> For 2nd & 3rd,
>> Acked-by: Jingoo 
>> But, I still want someone to ack 1st patch, not me.
>>
>> To Vidya,
>> If possible, can you ask your coworker to give 'Tested-by'? It will be very 
>> helpful.
>> Thank you.
> 
> On next-20201026 (but also going back quite a while) I'm seeing this
> during boot on Jetson AGX Xavier (Tegra194):
> 
> [3.493382] ahci 0001:01:00.0: version 3.0
> [3.493889] ahci 0001:01:00.0: SSS flag set, parallel bus scan disabled
> [4.497706] ahci 0001:01:00.0: controller reset failed (0x)
> [4.498114] ahci: probe of 0001:01:00.0 failed with error -5
> 
> After applying this series, AHCI over PCI is back to normal:
> 
> [3.543230] ahci 0001:01:00.0: AHCI 0001. 32 slots 1 ports 6 Gbps 0x1 
> impl SATA mode
> [3.550841] ahci 0001:01:00.0: flags: 64bit ncq sntf led only pmp fbs pio 
> slum part sxs
> [3.559747] scsi host0: ahci
> [3.561998] ata1: SATA max UDMA/133 abar m512@0x123001 port 
> 0x1230010100 irq 63
> 
> So for the series:
> 
> Tested-by: Thierry Reding 

FWIW ...

Tested-by: Jon Hunter 

Cheers
Jon

-- 
nvpublic

[PATCH V2] drm/tegra: sor: Don't warn on probe deferral

[Jon Hunter]

Deferred probe is an expected return value for tegra_output_probe().
Given that the driver deals with it properly, there's no need to output
a warning that may potentially confuse users.

Signed-off-by: Jon Hunter 
---

Changes since V1:
- This time, I actually validated it!

 drivers/gpu/drm/tegra/sor.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
index e88a17c2937f..898a80ca37fa 100644
--- a/drivers/gpu/drm/tegra/sor.c
+++ b/drivers/gpu/drm/tegra/sor.c
@@ -3764,10 +3764,9 @@ static int tegra_sor_probe(struct platform_device *pdev)
return err;
 
err = tegra_output_probe(>output);
-   if (err < 0) {
-   dev_err(>dev, "failed to probe output: %d\n", err);
-   return err;
-   }
+   if (err < 0)
+   return dev_err_probe(>dev, err,
+"failed to probe output: %d\n", err);
 
if (sor->ops && sor->ops->probe) {
err = sor->ops->probe(sor);
-- 
2.25.1

Re: [PATCH] drm/tegra: sor: Don't warn on probe deferral

[Jon Hunter]

On 03/11/2020 11:44, Jon Hunter wrote:
> Deferred probe is an expected return value for tegra_output_probe().
> Given that the driver deals with it properly, there's no need to output
> a warning that may potentially confuse users.
> 
> Signed-off-by: Jon Hunter 
> ---
>  drivers/gpu/drm/tegra/sor.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
> index e88a17c2937f..5a232055b8cc 100644
> --- a/drivers/gpu/drm/tegra/sor.c
> +++ b/drivers/gpu/drm/tegra/sor.c
> @@ -3765,7 +3765,7 @@ static int tegra_sor_probe(struct platform_device *pdev)
>  
>   err = tegra_output_probe(>output);
>   if (err < 0) {
> - dev_err(>dev, "failed to probe output: %d\n", err);
> + dev_err_probe(>dev, "failed to probe output: %d\n", err);
>   return err;
>   }

Sorry this is not right. I will fix this in V2.

Jon

-- 
nvpublic

[PATCH] cpufreq: tegra186: Fix get frequency callback

[Jon Hunter]

Commit b89c01c96051 ("cpufreq: tegra186: Fix initial frequency")
implemented the CPUFREQ 'get' callback to determine the current
operating frequency for each CPU. This implementation used a simple
looked up to determine the current operating frequency. The problem
with this is that frequency table for different Tegra186 devices may
vary and so the default boot frequency for Tegra186 device may or may
not be present in the frequency table. If the default boot frequency is
not present in the frequency table, this causes the function
tegra186_cpufreq_get() to return 0 and in turn causes cpufreq_online()
to fail which prevents CPUFREQ from working.

Fix this by always calculating the CPU frequency based upon the current
'ndiv' setting for the CPU. Note that the CPU frequency for Tegra186 is
calculated by reading the current 'ndiv' setting, multiplying by the
CPU reference clock and dividing by a constant divisor.

Fixes: b89c01c96051 ("cpufreq: tegra186: Fix initial frequency")

Signed-off-by: Jon Hunter 
---
 drivers/cpufreq/tegra186-cpufreq.c | 33 +++---
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/drivers/cpufreq/tegra186-cpufreq.c 
b/drivers/cpufreq/tegra186-cpufreq.c
index 4b4079f51559..7eb2c56c65de 100644
--- a/drivers/cpufreq/tegra186-cpufreq.c
+++ b/drivers/cpufreq/tegra186-cpufreq.c
@@ -42,6 +42,8 @@ static const struct tegra186_cpufreq_cluster_info 
tegra186_clusters[] = {
 struct tegra186_cpufreq_cluster {
const struct tegra186_cpufreq_cluster_info *info;
struct cpufreq_frequency_table *table;
+   u32 ref_clk_khz;
+   u32 div;
 };
 
 struct tegra186_cpufreq_data {
@@ -94,7 +96,7 @@ static int tegra186_cpufreq_set_target(struct cpufreq_policy 
*policy,
 
 static unsigned int tegra186_cpufreq_get(unsigned int cpu)
 {
-   struct cpufreq_frequency_table *tbl;
+   struct tegra186_cpufreq_data *data = cpufreq_get_driver_data();
struct cpufreq_policy *policy;
void __iomem *edvd_reg;
unsigned int i, freq = 0;
@@ -104,17 +106,23 @@ static unsigned int tegra186_cpufreq_get(unsigned int cpu)
if (!policy)
return 0;
 
-   tbl = policy->freq_table;
edvd_reg = policy->driver_data;
ndiv = readl(edvd_reg) & EDVD_CORE_VOLT_FREQ_F_MASK;
 
-   for (i = 0; tbl[i].frequency != CPUFREQ_TABLE_END; i++) {
-   if ((tbl[i].driver_data & EDVD_CORE_VOLT_FREQ_F_MASK) == ndiv) {
-   freq = tbl[i].frequency;
-   break;
+   for (i = 0; i < data->num_clusters; i++) {
+   struct tegra186_cpufreq_cluster *cluster = >clusters[i];
+   int core;
+
+   for (core = 0; core < ARRAY_SIZE(cluster->info->cpus); core++) {
+   if (cluster->info->cpus[core] != policy->cpu)
+   continue;
+
+   freq = (cluster->ref_clk_khz * ndiv) / cluster->div;
+   goto out;
}
}
 
+out:
cpufreq_cpu_put(policy);
 
return freq;
@@ -133,7 +141,7 @@ static struct cpufreq_driver tegra186_cpufreq_driver = {
 
 static struct cpufreq_frequency_table *init_vhint_table(
struct platform_device *pdev, struct tegra_bpmp *bpmp,
-   unsigned int cluster_id)
+   struct tegra186_cpufreq_cluster *cluster)
 {
struct cpufreq_frequency_table *table;
struct mrq_cpu_vhint_request req;
@@ -152,7 +160,7 @@ static struct cpufreq_frequency_table *init_vhint_table(
 
memset(, 0, sizeof(req));
req.addr = phys;
-   req.cluster_id = cluster_id;
+   req.cluster_id = cluster->info->bpmp_cluster_id;
 
memset(, 0, sizeof(msg));
msg.mrq = MRQ_CPU_VHINT;
@@ -185,6 +193,9 @@ static struct cpufreq_frequency_table *init_vhint_table(
goto free;
}
 
+   cluster->ref_clk_khz = data->ref_clk_hz / 1000;
+   cluster->div = data->pdiv * data->mdiv;
+
for (i = data->vfloor, j = 0; i <= data->vceil; i++) {
struct cpufreq_frequency_table *point;
u16 ndiv = data->ndiv[i];
@@ -202,8 +213,7 @@ static struct cpufreq_frequency_table *init_vhint_table(
 
point = [j++];
point->driver_data = edvd_val;
-   point->frequency = data->ref_clk_hz * ndiv / data->pdiv /
-   data->mdiv / 1000;
+   point->frequency = (cluster->ref_clk_khz * ndiv) / cluster->div;
}
 
table[j].frequency = CPUFREQ_TABLE_END;
@@ -245,8 +255,7 @@ static int tegra186_cpufreq_probe(struct platform_device 
*pdev)
struct tegra186_cpufreq_cluster *cluster = >clusters[i];
 
cluster->info = _clusters[i];
-   cluster->table = init_vhint_table(
-   pdev, bpmp, cluster->i

[PATCH] drm/tegra: sor: Don't warn on probe deferral

[Jon Hunter]

Deferred probe is an expected return value for tegra_output_probe().
Given that the driver deals with it properly, there's no need to output
a warning that may potentially confuse users.

Signed-off-by: Jon Hunter 
---
 drivers/gpu/drm/tegra/sor.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
index e88a17c2937f..5a232055b8cc 100644
--- a/drivers/gpu/drm/tegra/sor.c
+++ b/drivers/gpu/drm/tegra/sor.c
@@ -3765,7 +3765,7 @@ static int tegra_sor_probe(struct platform_device *pdev)
 
err = tegra_output_probe(>output);
if (err < 0) {
-   dev_err(>dev, "failed to probe output: %d\n", err);
+   dev_err_probe(>dev, "failed to probe output: %d\n", err);
return err;
}
 
-- 
2.25.1

Re: [PATCH 5.9 000/757] 5.9.2-rc1 review

[Jon Hunter]

On 27/10/2020 13:44, Greg Kroah-Hartman wrote:
> This is the start of the stable review cycle for the 5.9.2 release.
> There are 757 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.
> 
> Responses should be made by Thu, 29 Oct 2020 13:52:54 +.
> Anything received after that time might be too late.
> 
> The whole patch series can be found in one patch at:
>   
> https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.9.2-rc1.gz
> or in the git tree and branch at:
>   
> git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git 
> linux-5.9.y
> and the diffstat can be found below.
> 
> thanks,
> 
> greg k-h

One known/expected failure, but all other tests passing for Tegra ...

Test results for stable-v5.9:
15 builds:  15 pass, 0 fail
26 boots:   26 pass, 0 fail
61 tests:   60 pass, 1 fail

Linux version:  5.9.2-rc1-ge0fc09529493
Boards tested:  tegra124-jetson-tk1, tegra186-p2771-,
tegra194-p2972-, tegra20-ventana,
tegra210-p2371-2180, tegra210-p3450-0000,
tegra30-cardhu-a04

Tested-by: Jon Hunter 

The above failure is fixed in the mainline by the following commit ...

commit 97148d0ae5303bcc18fcd1c9b968a9485292f32a
Author: Viresh Kumar 
Date:   Tue Oct 13 10:42:47 2020 +0530

cpufreq: Improve code around unlisted freq check


Let me know if we can pull into linux-5.9.y.

Cheers!
Jon

-- 
nvpublic

Re: [PATCH V2] cpufreq: tegra186: Fix initial frequency

[Jon Hunter]

On 28/10/2020 04:11, Viresh Kumar wrote:
> On 26-10-20, 12:57, Jon Hunter wrote:
>> Thinking about this some more, what are your thoughts on making the
>> following change? 
>>
>> Basically, if the driver sets the CPUFREQ_NEED_INITIAL_FREQ_CHECK,
> 
> This flag only means that the platform would like the core to check
> the currently programmed frequency and get it in sync with the table.

Yes exactly.

>> then I wonder if we should not fail if the frequency return by
>>> get() is not known.
> 
> When do we fail if the frequency isn't known ? That's the case where
> we try to set it to one from the table.

Currently, if the frequency is not known, we fail right before we do the
initial frequency check [0].

> But (looking at your change), ->get() can't really return 0. We depend
> on it to get us the exact frequency the hardware is programmed at
> instead of reading a cached value in the software.

Actually it can and it does currently. Note in tegra186_cpufreq_get()
the variable 'freq' is initialised to 0, and if no match is found, then
it returns 0. This is what happens currently on some Tegra186 boards.

>>> This would fix the problem I see on Tegra186
>> where the initial boot frequency may not be in the frequency table.
> 
> With current mainline, what's the problem you see now ? Sorry I missed
> track of it a bit :)

No problem, this has been an on-going saga now for sometime.

Cheers
Jon

[0]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/cpufreq/cpufreq.c#n1429
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/cpufreq/tegra186-cpufreq.c#n95

-- 
nvpublic

Re: [PATCH] [v2] firmware: tegra: fix strncpy()/strncat() confusion

[Jon Hunter]

On 26/10/2020 16:49, Arnd Bergmann wrote:
> From: Arnd Bergmann 
> 
> The way that bpmp_populate_debugfs_inband() uses strncpy()
> and strncat() makes no sense since the size argument for
> the first is insufficient to contain the trailing '/'

I don't believe that is the case, because there is a +1 for trailing '/'
and the if statement is checking if the len is equal to or greater than.
If it is equal then there is no room for the nul character and will
fail. So it should not overflow.

> and the second passes the length of the input rather than
> the output, which triggers a warning:
> 
> In function 'strncat',
> inlined from 'bpmp_populate_debugfs_inband' at 
> ../drivers/firmware/tegra/bpmp-debugfs.c:422:4:
> include/linux/string.h:289:30: warning: '__builtin_strncat' specified bound 
> depends on the length of the source argument [-Wstringop-overflow=]
>   289 | #define __underlying_strncat __builtin_strncat
>   |  ^
> include/linux/string.h:367:10: note: in expansion of macro 
> '__underlying_strncat'
>   367 |   return __underlying_strncat(p, q, count);
>   |  ^~~~
> drivers/firmware/tegra/bpmp-debugfs.c: In function 
> 'bpmp_populate_debugfs_inband':
> include/linux/string.h:288:29: note: length computed here
>   288 | #define __underlying_strlen __builtin_strlen
>   | ^
> include/linux/string.h:321:10: note: in expansion of macro 
> '__underlying_strlen'
>   321 |   return __underlying_strlen(p);
> 
> Simplify this to use an snprintf() instead.
> 
> Fixes: 5e37b9c137ee ("firmware: tegra: Add support for in-band debug")
> Signed-off-by: Arnd Bergmann 
> ---
> v2: Use the correct arguments for snprintf(), as pointed out by Arvind Sankar
> ---
>  drivers/firmware/tegra/bpmp-debugfs.c | 6 +-
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/firmware/tegra/bpmp-debugfs.c 
> b/drivers/firmware/tegra/bpmp-debugfs.c
> index c1bbba9ee93a..440d99c63638 100644
> --- a/drivers/firmware/tegra/bpmp-debugfs.c 
> +++ b/drivers/firmware/tegra/bpmp-debugfs.c
> @@ -412,16 +412,12 @@ static int bpmp_populate_debugfs_inband(struct 
> tegra_bpmp *bpmp,
>   goto out;
>   }
>  
> - len = strlen(ppath) + strlen(name) + 1;
> + len = snprintf(pathbuf, pathlen, "%s%s/", ppath, name);
>   if (len >= pathlen) {
>   err = -EINVAL;
>   goto out;
>   }
>  
> - strncpy(pathbuf, ppath, pathlen);
> - strncat(pathbuf, name, strlen(name));
> - strcat(pathbuf, "/");
> -
>   err = bpmp_populate_debugfs_inband(bpmp, dentry,
>  pathbuf);
>   if (err < 0)
> 

However, this is indeed much better and so thanks for the simplification.

Acked-by: Jon Hunter 

Cheers
Jon

-- 
nvpublic

Re: [PATCH V2] cpufreq: tegra186: Fix initial frequency

[Jon Hunter]

On 19/10/2020 10:33, Jon Hunter wrote:
> 
> On 16/10/2020 05:07, Viresh Kumar wrote:
>> On 15-10-20, 15:03, Jon Hunter wrote:
>>> If not too late, would you mind dropping this patch for v5.10?
>>
>> It is already part of Linus's master now.
> 
> OK, thanks. I will send a revert for this once rc1 is out.


Thinking about this some more, what are your thoughts on making the
following change? 

Basically, if the driver sets the CPUFREQ_NEED_INITIAL_FREQ_CHECK,
then I wonder if we should not fail if the frequency return by
>get() is not known. This would fix the problem I see on Tegra186
where the initial boot frequency may not be in the frequency table.

Cheers
Jon

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index f4b60663efe6..b7d3b61577b0 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1426,13 +1426,8 @@ static int cpufreq_online(unsigned int cpu)
CPUFREQ_CREATE_POLICY, policy);
}
 
-   if (cpufreq_driver->get && has_target()) {
+   if (cpufreq_driver->get && has_target())
policy->cur = cpufreq_driver->get(policy->cpu);
-   if (!policy->cur) {
-   pr_err("%s: ->get() failed\n", __func__);
-   goto out_destroy_policy;
-   }
-   }
 
/*
 * Sometimes boot loaders set CPU frequency to a value outside of
@@ -1471,6 +1466,11 @@ static int cpufreq_online(unsigned int cpu)
pr_info("%s: CPU%d: Running at unlisted initial 
frequency: %u KHz, changing to: %u KHz\n",
__func__, policy->cpu, old_freq, policy->cur);
}
+   } else {
+   if (!policy->cur) {
+   pr_err("%s: ->get() failed\n", __func__);
+   goto out_destroy_policy;
+   }
}
 
if (new_policy) {

-- 
nvpublic

Re: [PATCH V2] cpufreq: tegra186: Fix initial frequency

[Jon Hunter]

On 16/10/2020 05:07, Viresh Kumar wrote:
> On 15-10-20, 15:03, Jon Hunter wrote:
>> If not too late, would you mind dropping this patch for v5.10?
> 
> It is already part of Linus's master now.

OK, thanks. I will send a revert for this once rc1 is out.

Cheers
Jon

-- 
nvpublic

Re: [PATCH 5.9 00/15] 5.9.1-rc1 review

[Jon Hunter]

On 16/10/2020 10:08, Greg Kroah-Hartman wrote:
> This is the start of the stable review cycle for the 5.9.1 release.
> There are 15 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.
> 
> Responses should be made by Sun, 18 Oct 2020 09:04:25 +.
> Anything received after that time might be too late.
> 
> The whole patch series can be found in one patch at:
>   
> https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.9.1-rc1.gz
> or in the git tree and branch at:
>   
> git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git 
> linux-5.9.y
> and the diffstat can be found below.
> 
> thanks,
> 
> greg k-h


There is one test failure (new warning) for Tegra194 but this is a known
issue for v5.9 and is fixed by Viresh's change [0]. Maybe we can pull
into stable once it is merged to the mainline?  

That said everything else looks good ...

Test results for stable-v5.9:
15 builds:  15 pass, 0 fail
26 boots:   26 pass, 0 fail
61 tests:   60 pass, 1 fail

Linux version:  5.9.1-rc1-g1cbc5f2d0eac
Boards tested:  tegra124-jetson-tk1, tegra186-p2771-,
tegra194-p2972-, tegra20-ventana,
tegra210-p2371-2180, tegra210-p3450-0000,
tegra30-cardhu-a04

Tested-by: Jon Hunter 

Jon

[0]
https://lore.kernel.org/linux-pm/37c3f1f76c055b305d1bba2c2001ac5b1d7a9b5f.1602565964.git.viresh.ku...@linaro.org/

-- 
nvpublic

Re: [PATCH V2] cpufreq: tegra186: Fix initial frequency

[Jon Hunter]

Hi Viresh,

On 25/08/2020 06:50, Viresh Kumar wrote:
> On 24-08-20, 15:59, Jon Hunter wrote:
>> Commit 6cc3d0e9a097 ("cpufreq: tegra186: add
>> CPUFREQ_NEED_INITIAL_FREQ_CHECK flag") fixed CPUFREQ support for
>> Tegra186 but as a consequence the following warnings are now seen on
>> boot ...
>>
>>  cpufreq: cpufreq_online: CPU0: Running at unlisted freq: 0 KHz
>>  cpufreq: cpufreq_online: CPU0: Unlisted initial frequency changed to: 
>> 2035200 KHz
>>  cpufreq: cpufreq_online: CPU1: Running at unlisted freq: 0 KHz
>>  cpufreq: cpufreq_online: CPU1: Unlisted initial frequency changed to: 
>> 2035200 KHz
>>  cpufreq: cpufreq_online: CPU2: Running at unlisted freq: 0 KHz
>>  cpufreq: cpufreq_online: CPU2: Unlisted initial frequency changed to: 
>> 2035200 KHz
>>  cpufreq: cpufreq_online: CPU3: Running at unlisted freq: 0 KHz
>>  cpufreq: cpufreq_online: CPU3: Unlisted initial frequency changed to: 
>> 2035200 KHz
>>  cpufreq: cpufreq_online: CPU4: Running at unlisted freq: 0 KHz
>>  cpufreq: cpufreq_online: CPU4: Unlisted initial frequency changed to: 
>> 2035200 KHz
>>  cpufreq: cpufreq_online: CPU5: Running at unlisted freq: 0 KHz
>>  cpufreq: cpufreq_online: CPU5: Unlisted initial frequency changed to: 
>> 2035200 KHz
>>
>> Fix this by adding a 'get' callback for the Tegra186 CPUFREQ driver to
>> retrieve the current operating frequency for a given CPU. The 'get'
>> callback uses the current 'ndiv' value that is programmed to determine
>> that current operating frequency.
>>
>> Signed-off-by: Jon Hunter 
>> ---
>> Changes since V1:
>> - Moved code into a 'get' callback
>>
>>  drivers/cpufreq/tegra186-cpufreq.c | 30 ++
>>  1 file changed, 30 insertions(+)
>>
>> diff --git a/drivers/cpufreq/tegra186-cpufreq.c 
>> b/drivers/cpufreq/tegra186-cpufreq.c
>> index 01e1f58ba422..0d0fcff60765 100644
>> --- a/drivers/cpufreq/tegra186-cpufreq.c
>> +++ b/drivers/cpufreq/tegra186-cpufreq.c
>> @@ -14,6 +14,7 @@
>>  
>>  #define EDVD_CORE_VOLT_FREQ(core)   (0x20 + (core) * 0x4)
>>  #define EDVD_CORE_VOLT_FREQ_F_SHIFT 0
>> +#define EDVD_CORE_VOLT_FREQ_F_MASK  0x
>>  #define EDVD_CORE_VOLT_FREQ_V_SHIFT 16
>>  
>>  struct tegra186_cpufreq_cluster_info {
>> @@ -91,10 +92,39 @@ static int tegra186_cpufreq_set_target(struct 
>> cpufreq_policy *policy,
>>  return 0;
>>  }
>>  
>> +static unsigned int tegra186_cpufreq_get(unsigned int cpu)
>> +{
>> +struct cpufreq_frequency_table *tbl;
>> +struct cpufreq_policy *policy;
>> +void __iomem *edvd_reg;
>> +unsigned int i, freq = 0;
>> +u32 ndiv;
>> +
>> +policy = cpufreq_cpu_get(cpu);
>> +if (!policy)
>> +return -EINVAL;
> 
> This should be return 0;
> 
> Applied with that change. Thanks.


If not too late, would you mind dropping this patch for v5.10?

The patch was working for me locally, but when testing on more boards, I
actually found this broke CPUFREQ support on some Tegra186 boards. The
reason is that the boot frequency may not be in the frequency table on
all boards. The boot frequency is constant across all boards, but the
frequency table can vary slightly with device. Therefore, for some
boards, such as mine, the boot frequency is found the in frequency table
but this is not always the case.

Now that you have made the warning an info print, this change is not
really needed and the simplest thing to do is dropped this completely.
Sorry about that.

Jon

-- 
nvpublic

Re: [PATCH 0/3] soc/tegra: Prevent the PMC driver from corrupting interrupt routing

[Jon Hunter]

On 05/10/2020 16:45, Thierry Reding wrote:

...

>>> Let Jon and myself do a bit of testing with this to verify that the wake
>>> up paths are still working.
>>
>> Sure. Let me know what you find.
> 
> The results are in and it's a bit of a mixed bag. I was able to confirm
> that Tegra194 also boots again after this series and I'm also able to
> resume from sleep using either rtcwake or the power-key as wakeup
> source, so the wake-events mechanism is still functional after the
> series. I do see a bit of breakage on resume, but none of that seems
> related to your patches and is likely something that crept in while we
> were looking into the current issue.
> 
> Jon had started a job in our test farm in parallel and that came back
> with a failing suspend/resume test on Tegra186 (Jetson TX2), but that
> seems to have been a pre-existing issue. This was already in linux-next
> around next-20200910 and Jon had been investigating it when the boot
> failures due to the IPI changes started happening. So I then hooked up
> my Jetson TX2 and verified locally that I can properly suspend/resume
> using either rtcwake or the power-key as wakeup source, just like I
> previously did on Tegra194 (Jetson AGX Xavier). Tegra186 seems to be a
> little more unstable because it didn't boot every time for me, but that
> is probably not related to this.

Yes my feeling is that those are other issues too that we need to look
at next.

> So, I'm tempted to say:
> 
> Tested-by: Thierry Reding 

Yes and you can have my ...

Tested-by: Jon Hunter 

Thanks again Marc for tracking this down!

Cheers
Jon

-- 
nvpublic

Re: [Patch 1/2] cpufreq: tegra194: get consistent cpuinfo_cur_freq

[Jon Hunter]

On 05/10/2020 05:34, Viresh Kumar wrote:
> On 17-09-20, 09:36, Jon Hunter wrote:
>> Viresh, ideally we need to include this fix for v5.9. Do you need Sumit
>> to resend with the Fixes tag or are you happy to add?
> 
> I understand that this fixes a patch which got merged recently, but I am not
> sure if anything is broken badly right now, i.e. will make the hardware work
> incorrectly.
> 
> Do we really need to get these in 5.9 ? As these are significant changes and 
> may
> cause more bugs. Won't getting them in 5.9-stable and 5.10-rc1 be enough ?


Yes we want these in v5.9 ideally but yes we could merge to 5.9-stable
once accepted into the mainline.

Jon

-- 
nvpublic

Re: [PATCH v3 10/13] ASoC: tegra: Add audio graph based card driver

[Jon Hunter]

On 01/10/2020 20:07, Michał Mirosław wrote:
> On Thu, Oct 01, 2020 at 11:03:04PM +0530, Sameer Pujar wrote:
>> Add Tegra audio machine driver which is based on generic audio graph card
>> driver. It re-uses most of the common stuff from audio graph driver and
>> uses the same DT binding. Required Tegra specific customizations are done
>> in the driver.
> [...]
>> +switch (srate) {
>> +case 11025:
>> +case 22050:
>> +case 44100:
>> +case 88200:
>> +case 176400:
>> +plla_out0_rate = chip_data->plla_out0_rates[x11_RATE];
>> +plla_rate = chip_data->plla_rates[x11_RATE];
>> +break;
>> +case 8000:
>> +case 16000:
>> +case 32000:
>> +case 48000:
>> +case 96000:
>> +case 192000:
> [...]
> 
> Do you really need to enumerate the frequencies? Wouldn't just checking
> srate % 11025 be enough to divide the set in two? Or just calculating
> the PLLA base rate by multiplying?


This is quite common among other ASoC drivers from what I can see. The
PLL rate does not scale with the srate, we just use a different PLL rate
depending on if the srate is 11025 Hz or 8000 Hz based. I don't see any
need to change the above.

Cheers
Jon

-- 
nvpublic

Re: [PATCH v2 0/5] PCI: dwc: improve msi handling

[Jon Hunter]

On 29/09/2020 18:25, Marc Zyngier wrote:
> On 2020-09-29 14:22, Jon Hunter wrote:
>> Hi Jisheng,
>>
>> On 29/09/2020 11:48, Jisheng Zhang wrote:
>>> Hi Jon,
>>>
>>> On Fri, 25 Sep 2020 09:53:45 +0100 Jon Hunter wrote:
>>>
>>>>
>>>> On 24/09/2020 12:05, Jisheng Zhang wrote:
>>>>> Improve the msi code:
>>>>> 1. Add proper error handling.
>>>>> 2. Move dw_pcie_msi_init() from each users to designware host to solve
>>>>> msi page leakage in resume path.
>>>>
>>>> Apologies if this is slightly off topic, but I have been meaning to ask
>>>> about MSIs and PCI. On Tegra194 which uses the DWC PCI driver,
>>>> whenever we
>>>> hotplug CPUs we see the following warnings ...
>>>>
>>>>  [  79.068351] WARNING KERN IRQ70: set affinity failed(-22).
>>>>  [  79.068362] WARNING KERN IRQ71: set affinity failed(-22).
>>>>
>>>
>>> I tried to reproduce this issue on Synaptics SoC, but can't reproduce
>>> it.
>>> Per my understanding of the code in kernel/irq/cpuhotplug.c, this
>>> warning
>>> happened when we migrate irqs away from the offline cpu, this implicitly
>>> implies that before this point the irq has bind to the offline cpu,
>>> but how
>>> could this happen given current dw_pci_msi_set_affinity() implementation
>>> always return -EINVAL
>>
>> By default the smp_affinity should be set so that all CPUs can be
>> interrupted ...
>>
>> $ cat /proc/irq/70/smp_affinity
>> 0xff
>>
>> In my case there are 8 CPUs and so 0xff implies that the interrupt can
>> be triggered on any of the 8 CPUs.
>>
>> Do you see the set_affinity callback being called for the DWC irqchip in
>> migrate_one_irq()?
> 
> The problem is common to all MSI implementations that end up muxing
> all the end-point MSIs into a single interrupt. With these systems,
> you cannot set the affinity of individual MSIs (they don't target a
> CPU, they target another interrupt... braindead). Only the mux
> interrupt can have its affinity changed.
> 
> So returning -EINVAL is the right thing to do.

Right, so if that is the case, then surely there should be some way to
avoid these warnings because they are not relevant?

Cheers
Jon

-- 
nvpublic

Re: [PATCH v2 0/5] PCI: dwc: improve msi handling

[Jon Hunter]

Hi Jisheng,

On 29/09/2020 11:48, Jisheng Zhang wrote:
> Hi Jon,
> 
> On Fri, 25 Sep 2020 09:53:45 +0100 Jon Hunter wrote:
> 
>>
>> On 24/09/2020 12:05, Jisheng Zhang wrote:
>>> Improve the msi code:
>>> 1. Add proper error handling.
>>> 2. Move dw_pcie_msi_init() from each users to designware host to solve
>>> msi page leakage in resume path.  
>>
>> Apologies if this is slightly off topic, but I have been meaning to ask
>> about MSIs and PCI. On Tegra194 which uses the DWC PCI driver, whenever we
>> hotplug CPUs we see the following warnings ...
>>
>>  [  79.068351] WARNING KERN IRQ70: set affinity failed(-22).
>>  [  79.068362] WARNING KERN IRQ71: set affinity failed(-22).
>>
> 
> I tried to reproduce this issue on Synaptics SoC, but can't reproduce it.
> Per my understanding of the code in kernel/irq/cpuhotplug.c, this warning
> happened when we migrate irqs away from the offline cpu, this implicitly
> implies that before this point the irq has bind to the offline cpu, but how
> could this happen given current dw_pci_msi_set_affinity() implementation
> always return -EINVAL

By default the smp_affinity should be set so that all CPUs can be
interrupted ...

$ cat /proc/irq/70/smp_affinity
0xff

In my case there are 8 CPUs and so 0xff implies that the interrupt can
be triggered on any of the 8 CPUs.

Do you see the set_affinity callback being called for the DWC irqchip in
migrate_one_irq()?

Cheers
Jon

-- 
nvpublic

Re: [PATCH v2 0/5] PCI: dwc: improve msi handling

[Jon Hunter]

On 27/09/2020 09:28, Jisheng Zhang wrote:

...

> I see, the msi_domain_set_affinity() calls parent->chip->irq_set_affinity
> without checking, grepping the irqchip and pci dir, I found that
> if the MSI is based on some cascaded interrupt mechanism, they all
> point the irq_set_affinity to irq_chip_set_affinity_parent(), so I believe
> below patch works:
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c 
> b/drivers/pci/controller/dwc/pcie-designware-host.c
> index bf25d783b5c5..093fba616736 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -137,12 +137,6 @@ static void dw_pci_setup_msi_msg(struct irq_data *d, 
> struct msi_msg *msg)
>   (int)d->hwirq, msg->address_hi, msg->address_lo);
>  }
>  
> -static int dw_pci_msi_set_affinity(struct irq_data *d,
> -const struct cpumask *mask, bool force)
> -{
> - return -EINVAL;
> -}
> -
>  static void dw_pci_bottom_mask(struct irq_data *d)
>  {
>   struct pcie_port *pp = irq_data_get_irq_chip_data(d);
> @@ -197,7 +191,7 @@ static struct irq_chip dw_pci_msi_bottom_irq_chip = {
>   .name = "DWPCI-MSI",
>   .irq_ack = dw_pci_bottom_ack,
>   .irq_compose_msi_msg = dw_pci_setup_msi_msg,
> - .irq_set_affinity = dw_pci_msi_set_affinity,
> + .irq_set_affinity = irq_chip_set_affinity_parent,
>   .irq_mask = dw_pci_bottom_mask,
>   .irq_unmask = dw_pci_bottom_unmask,
>  };
> 


Unfortunately, this still crashes ...

[   11.521674] Unable to handle kernel NULL pointer dereference at virtual 
address 0018
[   11.530324] Mem abort info:
[   11.533089]   ESR = 0x9604
[   11.536105]   EC = 0x25: DABT (current EL), IL = 32 bits
[   11.541333]   SET = 0, FnV = 0
[   11.544344]   EA = 0, S1PTW = 0
[   11.547441] Data abort info:
[   11.550279]   ISV = 0, ISS = 0x0004
[   11.554056]   CM = 0, WnR = 0
[   11.557007] user pgtable: 4k pages, 48-bit VAs, pgdp=000467341000
[   11.56] [0018] pgd=, p4d=
[   11.570024] Internal error: Oops: 9604 [#1] PREEMPT SMP
[   11.575517] Modules linked in: crct10dif_ce pwm_tegra snd_hda_core 
phy_tegra194_p2u lm90 pcie_tegra194 tegra_bpmp_thermal pwm_fan ip_tables 
x_tables ipv6
[   11.589046] CPU: 3 PID: 148 Comm: kworker/3:1 Not tainted 
5.9.0-rc4-9-g6fdf18edb995-dirty #7
[   11.597669] Hardware name: NVIDIA Jetson AGX Xavier Developer Kit (DT)
[   11.604110] Workqueue: events deferred_probe_work_func
[   11.609174] pstate: 60c00089 (nZCv daIf +PAN +UAO BTYPE=--)
[   11.614657] pc : irq_chip_set_affinity_parent+0x4/0x30
[   11.619735] lr : msi_domain_set_affinity+0x44/0xc0
[   11.624448] sp : 800012d4b390
[   11.627744] x29: 800012d4b390 x28: 0003e7234c20 
[   11.632983] x27: 0003e913e460 x26:  
[   11.638231] x25: 800011d7e890 x24: 800011d7e8b8 
[   11.643466] x23:  x22: 0003e913e400 
[   11.648701] x21: 0003e913e460 x20: 0003e913e460 
[   11.653932] x19: 800011b19000 x18:  
[   11.659160] x17:  x16:  
[   11.664390] x15: 0001 x14: 0040 
[   11.669636] x13: 0228 x12: 0030 
[   11.674864] x11: 0101010101010101 x10: 0040 
[   11.680111] x9 :  x8 : 0004 
[   11.685363] x7 :  x6 : 00ff 
[   11.690596] x5 :  x4 :  
[   11.695843] x3 : 8000100d89a8 x2 :  
[   11.701058] x1 : 800011d7e8d8 x0 :  
[   11.706288] Call trace:
[   11.708708]  irq_chip_set_affinity_parent+0x4/0x30
[   11.713431]  irq_do_set_affinity+0x4c/0x178
[   11.717540]  irq_setup_affinity+0x124/0x1b0
[   11.721650]  irq_startup+0x6c/0x118
[   11.725081]  __setup_irq+0x810/0x8a0
[   11.728580]  request_threaded_irq+0xdc/0x188
[   11.732793]  pcie_pme_probe+0x98/0x110
[   11.736481]  pcie_port_probe_service+0x34/0x60
[   11.740848]  really_probe+0x110/0x400
[   11.75]  driver_probe_device+0x54/0xb8
[   11.748482]  __device_attach_driver+0x90/0xc0
[   11.752758]  bus_for_each_drv+0x70/0xc8
[   11.756526]  __device_attach+0xec/0x150
[   11.760306]  device_initial_probe+0x10/0x18
[   11.764413]  bus_probe_device+0x94/0xa0
[   11.768203]  device_add+0x464/0x730
[   11.771630]  device_register+0x1c/0x28
[   11.775311]  pcie_port_device_register+0x2d0/0x3e8
[   11.780017]  pcie_portdrv_probe+0x34/0xd8
[   11.783957]  local_pci_probe+0x3c/0xa0
[   11.787647]  pci_device_probe+0x128/0x1c8
[   11.791588]  really_probe+0x110/0x400
[   11.795179]  driver_probe_device+0x54/0xb8
[   11.799202]  __device_attach_driver+0x90/0xc0
[   11.803480]  bus_for_each_drv+0x70/0xc8
[   11.807244]  __device_attach+0xec/0x150
[   11.811009]  device_attach+0x10/0x18
[   11.814518]  pci_bus_add_device+0x4c/0xb0
[   11.818456]  pci_bus_add_devices+0x44/0x90
[   

Re: [PATCH v2 0/5] PCI: dwc: improve msi handling

[Jon Hunter]

Hi Jisheng,

On 25/09/2020 10:27, Jisheng Zhang wrote:

...

>> Could you please try below patch?
>>
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c 
>> b/drivers/pci/controller/dwc/pcie-designware-host.c
>> index bf25d783b5c5..7e5dc54d060e 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
>> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
>> @@ -197,7 +197,6 @@ static struct irq_chip dw_pci_msi_bottom_irq_chip = {
>> .name = "DWPCI-MSI",
>> .irq_ack = dw_pci_bottom_ack,
>> .irq_compose_msi_msg = dw_pci_setup_msi_msg,
>> -   .irq_set_affinity = dw_pci_msi_set_affinity,
>> .irq_mask = dw_pci_bottom_mask,
>> .irq_unmask = dw_pci_bottom_unmask,
>>  };
> 
> A complete patch w/o compiler warning:
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c 
> b/drivers/pci/controller/dwc/pcie-designware-host.c
> index bf25d783b5c5..18f719cfed0b 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -137,12 +137,6 @@ static void dw_pci_setup_msi_msg(struct irq_data *d, 
> struct msi_msg *msg)
>   (int)d->hwirq, msg->address_hi, msg->address_lo);
>  }
>  
> -static int dw_pci_msi_set_affinity(struct irq_data *d,
> -const struct cpumask *mask, bool force)
> -{
> - return -EINVAL;
> -}
> -
>  static void dw_pci_bottom_mask(struct irq_data *d)
>  {
>   struct pcie_port *pp = irq_data_get_irq_chip_data(d);
> @@ -197,7 +191,6 @@ static struct irq_chip dw_pci_msi_bottom_irq_chip = {
>   .name = "DWPCI-MSI",
>   .irq_ack = dw_pci_bottom_ack,
>   .irq_compose_msi_msg = dw_pci_setup_msi_msg,
> - .irq_set_affinity = dw_pci_msi_set_affinity,
>   .irq_mask = dw_pci_bottom_mask,
>   .irq_unmask = dw_pci_bottom_unmask,
>  };
> 


Thanks I was not expecting this to work because ...

 int irq_do_set_affinity(struct irq_data *data, const struct cpumask *mask,
 bool force)
 {
 struct irq_desc *desc = irq_data_to_desc(data);
 struct irq_chip *chip = irq_data_get_irq_chip(data);
 int ret;
 
 if (!chip || !chip->irq_set_affinity)
 return -EINVAL;

However, with your patch Tegra crashes on boot ...

[   11.613853] Unable to handle kernel NULL pointer dereference at virtual 
address 
[   11.622500] Mem abort info:
[   11.622515]   ESR = 0x8604
[   11.622524]   EC = 0x21: IABT (current EL), IL = 32 bits
[   11.622540]   SET = 0, FnV = 0
[   11.636544]   EA = 0, S1PTW = 0
[   11.636554] user pgtable: 4k pages, 48-bit VAs, pgdp=00046a28e000
[   11.636559] [] pgd=, p4d=
[   11.652652] Internal error: Oops: 8604 [#1] PREEMPT SMP
[   11.652658] Modules linked in: pwm_tegra phy_tegra194_p2u crct10dif_ce lm90 
pwm_fan tegra_bpmp_thermal pcie_tegra194 ip_tables x_tables ipv6
[   11.670525] CPU: 3 PID: 138 Comm: kworker/3:3 Not tainted 5.9.0-rc4-dirty #12
[   11.670534] Hardware name: NVIDIA Jetson AGX Xavier Developer Kit (DT)
[   11.683967] Workqueue: events deferred_probe_work_func
[   11.683974] pstate: 60c00089 (nZCv daIf +PAN +UAO BTYPE=--)
[   11.683985] pc : 0x0
[   11.696669] lr : msi_domain_set_affinity+0x44/0xc0
[   11.696672] sp : 800012bcb390
[   11.696680] x29: 800012bcb390 x28: 0003e3033c20 
[   11.709891] x27: 0003e76cfe58 x26:  
[   11.709900] x25: 800011d7e850 x24: 800011d7e878 
[   11.709908] x23:  x22: 0003e76cfe00 
[   11.709914] x21: 0003e76cfe58 x20: 0003e76cfe58 
[   11.709921] x19: 800011b19000 x18:  
[   11.709927] x17:  x16:  
[   11.741262] x15: 800011b19948 x14: 0040 
[   11.741267] x13: 0228 x12: 0030 
[   11.741272] x11: 0101010101010101 x10: 0040 
[   11.741277] x9 :  x8 : 0004 
[   11.741281] x7 :  x6 : 00ff 
[   11.767374] x5 :  x4 :  
[   11.767379] x3 :  x2 :  
[   11.767384] x1 : 800011d7e898 x0 : 0003e262bf00 
[   11.767406] Call trace:
[   11.767410]  0x0
[   11.767424]  irq_do_set_affinity+0x4c/0x178
[   11.791400]  irq_setup_affinity+0x124/0x1b0
[   11.791423]  irq_startup+0x6c/0x118
[   11.791434]  __setup_irq+0x810/0x8a0
[   11.802510]  request_threaded_irq+0xdc/0x188
[   11.802517]  pcie_pme_probe+0x98/0x110
[   11.802536]  pcie_port_probe_service+0x34/0x60
[   11.814799]  really_probe+0x110/0x400
[   11.814809]  driver_probe_device+0x54/0xb8
[   11.822438]  __device_attach_driver+0x90/0xc0
[   11.822463]  bus_for_each_drv+0x70/0xc8
[   11.822471]  __device_attach+0xec/0x150
[   11.834307]  device_initial_probe+0x10/0x18
[   11.834311]  bus_probe_device+0x94/0xa0
[   11.834315]  device_add+0x464/0x730
[   11.834338]  

Re: [PATCH v2 0/5] PCI: dwc: improve msi handling

[Jon Hunter]

On 24/09/2020 12:05, Jisheng Zhang wrote:
> Improve the msi code:
> 1. Add proper error handling.
> 2. Move dw_pcie_msi_init() from each users to designware host to solve
> msi page leakage in resume path.

Apologies if this is slightly off topic, but I have been meaning to ask
about MSIs and PCI. On Tegra194 which uses the DWC PCI driver, whenever we
hotplug CPUs we see the following warnings ...

 [  79.068351] WARNING KERN IRQ70: set affinity failed(-22).
 [  79.068362] WARNING KERN IRQ71: set affinity failed(-22).

These interrupts are the MSIs ...

70:  0  0  0  0  0  0  
0  0   PCI-MSI 134217728 Edge  PCIe PME, aerdrv
71:  0  0  0  0  0  0  
0  0   PCI-MSI 134742016 Edge  ahci[0001:01:00.0]

This caused because ...

 static int dw_pci_msi_set_affinity(struct irq_data *d,
const struct cpumask *mask, bool force)
 {
 return -EINVAL;
 }

Now the above is not unique to the DWC PCI host driver, it appears that
most PCIe drivers also do the same. However, I am curious if there is
any way to avoid the above warnings given that setting the affinity does
not appear to be supported in anyway AFAICT.

Cheers
Jon 

-- 
nvpublic

Re: [PATCH V2 0/5] Add support for custom names for AT24 EEPROMs

[Jon Hunter]

On 23/09/2020 10:15, Bartosz Golaszewski wrote:
> On Wed, Sep 16, 2020 at 11:50 AM Jon Hunter  wrote:
>>
>> For platforms that have multiple boards and hence have multiple EEPROMs
>> for identifying the different boards, it is useful to label the EEPROMs
>> in device-tree so that they can be easily identified. For example, MAC
>> address information is stored in the EEPROM on the processor module for
>> some Jetson platforms which is not only required by the kernel but the
>> bootloader as well. So having a simple way to identify the EEPROM is
>> needed.
>>
>> Changes since V1:
>> - By default initialise the nvmem_config.id as NVMEM_DEVID_AUTO and not
>>   NVMEM_DEVID_NONE
>> - Dropped the 'maxItems' from the dt-binding doc.
>>
>> Jon Hunter (5):
>>   misc: eeprom: at24: Initialise AT24 NVMEM ID field
>>   dt-bindings: eeprom: at24: Add label property for AT24
>>   misc: eeprom: at24: Support custom device names for AT24 EEPROMs
>>   arm64: tegra: Add label properties for EEPROMs
>>   arm64: tegra: Populate EEPROMs for Jetson Xavier NX
>>
>>  .../devicetree/bindings/eeprom/at24.yaml  |  3 +++
>>  .../boot/dts/nvidia/tegra186-p2771-.dts   |  1 +
>>  .../arm64/boot/dts/nvidia/tegra186-p3310.dtsi |  1 +
>>  .../arm64/boot/dts/nvidia/tegra194-p2888.dtsi |  1 +
>>  .../boot/dts/nvidia/tegra194-p2972-.dts   |  1 +
>>  .../nvidia/tegra194-p3509-+p3668-.dts | 14 
>>  .../boot/dts/nvidia/tegra194-p3668-.dtsi  | 16 ++
>>  .../arm64/boot/dts/nvidia/tegra210-p2180.dtsi |  1 +
>>  .../boot/dts/nvidia/tegra210-p2371-2180.dts   |  1 +
>>  .../boot/dts/nvidia/tegra210-p3450-.dts   |  2 ++
>>  drivers/misc/eeprom/at24.c| 22 ++-
>>  11 files changed, 62 insertions(+), 1 deletion(-)
>>
>> --
>> 2.25.1
>>
> 
> Just FYI: I'm fine with the at24 part. I can take them through my tree
> for v5.10. Who is taking the DTS patches for tegra? Thierry? I can
> provide you with an immutable branch if that's fine. I can't just ack
> the at24 patches because they conflict with what I already have in my
> tree for v5.10.

Thanks. Yes Thierry is picking up the DTS patches.

Cheers
Jon

-- 
nvpublic

Re: [Patch 1/2] cpufreq: tegra194: get consistent cpuinfo_cur_freq

[Jon Hunter]

Hi Rafael, Sudeep,

On 17/09/2020 09:44, Jon Hunter wrote:
> Adding Sudeep ...
> 
> On 17/09/2020 09:36, Jon Hunter wrote:
>>
>>
>> On 16/09/2020 18:11, Sumit Gupta wrote:
>>> Frequency returned by 'cpuinfo_cur_freq' using counters is not fixed
>>> and keeps changing slightly. This change returns a consistent value
>>> from freq_table. If the reconstructed frequency has acceptable delta
>>> from the last written value, then return the frequency corresponding
>>> to the last written ndiv value from freq_table. Otherwise, print a
>>> warning and return the reconstructed freq.
>>
>> We should include the Fixes tag here ...
>>
>> Fixes: df320f89359c ("cpufreq: Add Tegra194 cpufreq driver")
>>
>>>
>>> Signed-off-by: Sumit Gupta 
>>
>> Otherwise ...
>>
>> Reviewed-by: Jon Hunter 
>> Tested-by: Jon Hunter 
>>
>> Viresh, ideally we need to include this fix for v5.9. Do you need Sumit
>> to resend with the Fixes tag or are you happy to add?
> 
> 
> Sudeep, Rafael, looks like Viresh is out of office until next month.
> Please let me know if we can pick up both this patch and following patch
> for v5.9.

Any chance we can get these patches into v5.9?

Thanks!
Jon

-- 
nvpublic