* [regression 4.20-rc1, bisected] cpufreq fails to register on db820c
@ 2018-11-06 1:29 Niklas Cassel
2018-11-06 14:37 ` Srinivas Kandagatla
0 siblings, 1 reply; 3+ messages in thread
From: Niklas Cassel @ 2018-11-06 1:29 UTC (permalink / raw)
To: bgolaszewski; +Cc: srinivas.kandagatla, viresh.kumar, linux-kernel
Hello Bartosz,
I've run into a regression on v4.20-rc1, where cpufreq on dragonboard 820c
fails to register.
I've bisected the issue to:
e888d445ac33 ("nvmem: resolve cells from DT at registration time")
If I revert that patch, cpufreq starts working on dragonboard 820c again.
Before your patch:
[ 5.052985] qcom_cpufreq_kryo_probe:110 speedbin_nvmem: ffff8000d4d51e00
[ 5.053133] qcom_cpufreq_kryo_probe:120 speedbin: ffff8000d4d51d00
[ 5.058895] qcom_cpufreq_kryo_probe:121 *speedbin: 0
After your patch:
[ 4.791664] qcom_cpufreq_kryo_probe:110 speedbin_nvmem: ffff8000d472ed00
[ 4.791815] qcom_cpufreq_kryo_probe:120 speedbin: ffff8000d451b300
[ 4.797575] qcom_cpufreq_kryo_probe:121 *speedbin: 12
The prints are debug prints that I've added locally:
--- a/drivers/cpufreq/qcom-cpufreq-kryo.c
+++ b/drivers/cpufreq/qcom-cpufreq-kryo.c
@@ -107,6 +107,7 @@ static int qcom_cpufreq_kryo_probe(struct platform_device *pdev)
}
speedbin_nvmem = of_nvmem_cell_get(np, NULL);
+ pr_err("%s:%d speedbin_nvmem: %lx\n", __func__, __LINE__, speedbin_nvmem);
of_node_put(np);
if (IS_ERR(speedbin_nvmem)) {
if (PTR_ERR(speedbin_nvmem) != -EPROBE_DEFER)
@@ -116,6 +117,8 @@ static int qcom_cpufreq_kryo_probe(struct platform_device *pdev)
}
speedbin = nvmem_cell_read(speedbin_nvmem, &len);
+ pr_err("%s:%d speedbin: %lx\n", __func__, __LINE__, speedbin);
+ pr_err("%s:%d *speedbin: %u\n", __func__, __LINE__, (unsigned int)(*speedbin));
nvmem_cell_put(speedbin_nvmem);
if (IS_ERR(speedbin))
return PTR_ERR(speedbin);
So it appears that the read from nvmem_cell_read()
gets another value after your commit.
This value is then given to dev_pm_opp_set_supported_hw(),
however, since this new value (12) from nvmem_cell_read()
doesn't have any matching opp-supported-hw in device tree,
cpufreq fails to register.
This error only happens when we read this new incorrect value
(12 instead of 0).
Any idea why nvmem_cell_read() returns a new value for the
same efuse?
Kind regards,
Niklas
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [regression 4.20-rc1, bisected] cpufreq fails to register on db820c
2018-11-06 1:29 [regression 4.20-rc1, bisected] cpufreq fails to register on db820c Niklas Cassel
@ 2018-11-06 14:37 ` Srinivas Kandagatla
2018-11-06 15:26 ` Niklas Cassel
0 siblings, 1 reply; 3+ messages in thread
From: Srinivas Kandagatla @ 2018-11-06 14:37 UTC (permalink / raw)
To: Niklas Cassel, bgolaszewski; +Cc: viresh.kumar, linux-kernel
Hi Niklas,
thanks for reporting this..
I found the issue, DT support seems to be totally broken in NVMEM.
Will send this fix once you can confirm it works for you!
Can you try this patch!
---------------------->cut<--------------------------
Author: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Date: Tue Nov 6 14:32:30 2018 +0000
nvmem: core: fix of_nvmem_cell_get()
NVMEM DT support seems to be totally broken after
e888d445ac33 ("nvmem: resolve cells from DT at registration time")
Fix this!
Fixes: e888d445ac33 ("nvmem: resolve cells from DT at registration
time")
Reported-by: Niklas Cassel <niklas.cassel@linaro.org>
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 9b18ce90f907..7ae8072f56ae 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -44,6 +44,7 @@ struct nvmem_cell {
int bytes;
int bit_offset;
int nbits;
+ struct device_node *np;
struct nvmem_device *nvmem;
struct list_head node;
};
@@ -530,6 +531,7 @@ static int nvmem_add_cells_from_of(struct
nvmem_device *nvmem)
return -ENOMEM;
cell->nvmem = nvmem;
+ cell->np = child;
cell->offset = be32_to_cpup(addr++);
cell->bytes = be32_to_cpup(addr);
cell->name = kasprintf(GFP_KERNEL, "%pOFn", child);
@@ -960,14 +962,13 @@ nvmem_cell_get_from_lookup(struct device *dev,
const char *con_id)
#if IS_ENABLED(CONFIG_OF)
static struct nvmem_cell *
-nvmem_find_cell_by_index(struct nvmem_device *nvmem, int index)
+nvmem_find_cell_by_node(struct nvmem_device *nvmem, struct device_node *np)
{
struct nvmem_cell *cell = NULL;
- int i = 0;
mutex_lock(&nvmem_mutex);
list_for_each_entry(cell, &nvmem->cells, node) {
- if (index == i++)
+ if (np == cell->np)
break;
}
mutex_unlock(&nvmem_mutex);
@@ -1001,7 +1002,6 @@ struct nvmem_cell *of_nvmem_cell_get(struct
device_node *np, const char *id)
cell_np = of_parse_phandle(np, "nvmem-cells", index);
if (!cell_np)
return ERR_PTR(-EINVAL);
-
nvmem_np = of_get_next_parent(cell_np);
if (!nvmem_np)
return ERR_PTR(-EINVAL);
@@ -1011,7 +1011,7 @@ struct nvmem_cell *of_nvmem_cell_get(struct
device_node *np, const char *id)
if (IS_ERR(nvmem))
return ERR_CAST(nvmem);
- cell = nvmem_find_cell_by_index(nvmem, index);
+ cell = nvmem_find_cell_by_node(nvmem, cell_np);
if (!cell) {
__nvmem_device_put(nvmem);
return ERR_PTR(-ENOENT);
---------------------->cut<--------------------------
On 06/11/18 01:29, Niklas Cassel wrote:
> Hello Bartosz,
>
> I've run into a regression on v4.20-rc1, where cpufreq on dragonboard 820c
> fails to register.
>
> I've bisected the issue to:
> e888d445ac33 ("nvmem: resolve cells from DT at registration time")
>
> If I revert that patch, cpufreq starts working on dragonboard 820c again.
>
>
> Before your patch:
> [ 5.052985] qcom_cpufreq_kryo_probe:110 speedbin_nvmem: ffff8000d4d51e00
> [ 5.053133] qcom_cpufreq_kryo_probe:120 speedbin: ffff8000d4d51d00
> [ 5.058895] qcom_cpufreq_kryo_probe:121 *speedbin: 0
>
>
> After your patch:
> [ 4.791664] qcom_cpufreq_kryo_probe:110 speedbin_nvmem: ffff8000d472ed00
> [ 4.791815] qcom_cpufreq_kryo_probe:120 speedbin: ffff8000d451b300
> [ 4.797575] qcom_cpufreq_kryo_probe:121 *speedbin: 12
>
>
> The prints are debug prints that I've added locally:
>
> --- a/drivers/cpufreq/qcom-cpufreq-kryo.c
> +++ b/drivers/cpufreq/qcom-cpufreq-kryo.c
> @@ -107,6 +107,7 @@ static int qcom_cpufreq_kryo_probe(struct platform_device *pdev)
> }
>
> speedbin_nvmem = of_nvmem_cell_get(np, NULL);
> + pr_err("%s:%d speedbin_nvmem: %lx\n", __func__, __LINE__, speedbin_nvmem);
> of_node_put(np);
> if (IS_ERR(speedbin_nvmem)) {
> if (PTR_ERR(speedbin_nvmem) != -EPROBE_DEFER)
> @@ -116,6 +117,8 @@ static int qcom_cpufreq_kryo_probe(struct platform_device *pdev)
> }
>
> speedbin = nvmem_cell_read(speedbin_nvmem, &len);
> + pr_err("%s:%d speedbin: %lx\n", __func__, __LINE__, speedbin);
> + pr_err("%s:%d *speedbin: %u\n", __func__, __LINE__, (unsigned int)(*speedbin));
> nvmem_cell_put(speedbin_nvmem);
> if (IS_ERR(speedbin))
> return PTR_ERR(speedbin);
>
>
> So it appears that the read from nvmem_cell_read()
> gets another value after your commit.
>
> This value is then given to dev_pm_opp_set_supported_hw(),
> however, since this new value (12) from nvmem_cell_read()
> doesn't have any matching opp-supported-hw in device tree,
> cpufreq fails to register.
>
> This error only happens when we read this new incorrect value
> (12 instead of 0).
> Any idea why nvmem_cell_read() returns a new value for the
> same efuse?
>
>
> Kind regards,
> Niklas
>
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [regression 4.20-rc1, bisected] cpufreq fails to register on db820c
2018-11-06 14:37 ` Srinivas Kandagatla
@ 2018-11-06 15:26 ` Niklas Cassel
0 siblings, 0 replies; 3+ messages in thread
From: Niklas Cassel @ 2018-11-06 15:26 UTC (permalink / raw)
To: Srinivas Kandagatla; +Cc: bgolaszewski, viresh.kumar, linux-kernel
On Tue, Nov 06, 2018 at 02:37:57PM +0000, Srinivas Kandagatla wrote:
> Hi Niklas,
>
> thanks for reporting this..
>
> I found the issue, DT support seems to be totally broken in NVMEM.
> Will send this fix once you can confirm it works for you!
>
> Can you try this patch!
> ---------------------->cut<--------------------------
>
> Author: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Date: Tue Nov 6 14:32:30 2018 +0000
>
> nvmem: core: fix of_nvmem_cell_get()
>
> NVMEM DT support seems to be totally broken after
> e888d445ac33 ("nvmem: resolve cells from DT at registration time")
> Fix this!
>
> Fixes: e888d445ac33 ("nvmem: resolve cells from DT at registration
> time")
> Reported-by: Niklas Cassel <niklas.cassel@linaro.org>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index 9b18ce90f907..7ae8072f56ae 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -44,6 +44,7 @@ struct nvmem_cell {
> int bytes;
> int bit_offset;
> int nbits;
> + struct device_node *np;
> struct nvmem_device *nvmem;
> struct list_head node;
> };
> @@ -530,6 +531,7 @@ static int nvmem_add_cells_from_of(struct nvmem_device
> *nvmem)
> return -ENOMEM;
>
> cell->nvmem = nvmem;
> + cell->np = child;
> cell->offset = be32_to_cpup(addr++);
> cell->bytes = be32_to_cpup(addr);
> cell->name = kasprintf(GFP_KERNEL, "%pOFn", child);
> @@ -960,14 +962,13 @@ nvmem_cell_get_from_lookup(struct device *dev, const
> char *con_id)
>
> #if IS_ENABLED(CONFIG_OF)
> static struct nvmem_cell *
> -nvmem_find_cell_by_index(struct nvmem_device *nvmem, int index)
> +nvmem_find_cell_by_node(struct nvmem_device *nvmem, struct device_node *np)
> {
> struct nvmem_cell *cell = NULL;
> - int i = 0;
>
> mutex_lock(&nvmem_mutex);
> list_for_each_entry(cell, &nvmem->cells, node) {
> - if (index == i++)
> + if (np == cell->np)
> break;
> }
> mutex_unlock(&nvmem_mutex);
> @@ -1001,7 +1002,6 @@ struct nvmem_cell *of_nvmem_cell_get(struct
> device_node *np, const char *id)
> cell_np = of_parse_phandle(np, "nvmem-cells", index);
> if (!cell_np)
> return ERR_PTR(-EINVAL);
> -
> nvmem_np = of_get_next_parent(cell_np);
> if (!nvmem_np)
> return ERR_PTR(-EINVAL);
> @@ -1011,7 +1011,7 @@ struct nvmem_cell *of_nvmem_cell_get(struct
> device_node *np, const char *id)
> if (IS_ERR(nvmem))
> return ERR_CAST(nvmem);
>
> - cell = nvmem_find_cell_by_index(nvmem, index);
> + cell = nvmem_find_cell_by_node(nvmem, cell_np);
> if (!cell) {
> __nvmem_device_put(nvmem);
> return ERR_PTR(-ENOENT);
>
Tested-by: Niklas Cassel <niklas.cassel@linaro.org>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-11-06 15:26 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-06 1:29 [regression 4.20-rc1, bisected] cpufreq fails to register on db820c Niklas Cassel
2018-11-06 14:37 ` Srinivas Kandagatla
2018-11-06 15:26 ` Niklas Cassel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).