On Thu, 2019-10-03 at 16:47 -0700, Florian Fainelli wrote: > On 10/3/19 12:39 PM, Nicolas Saenz Julienne wrote: > > On Thu, 2019-10-03 at 11:08 -0700, Florian Fainelli wrote: > > > On 10/2/19 4:45 AM, Nicolas Saenz Julienne wrote: > > > > Currently, in arm_dt_init_cpu_maps(), the hwid of the boot CPU is read > > > > from MPIDR on SMP devices and set to 0 for non SMP. This value is then > > > > matched with the DT cpu nodes' reg property in order to find the boot > > > > CPU in DT. > > > > > > The code you change is about the "mpidr" variable, yet in your commit > > > message you refer to "hwid", that is a tad confusing for the reader. > > > > Sorry, it's indeed pretty confusing. I'll send a new version with a fixed > > description if there is no major push back. > > > > > > On MP devices build without SMP the cpu DT node contains the expected > > > > MPIDR yet the hwid is set to 0. With this the function fails to match > > > > the cpus and uses the default CPU logical map. Making it impossible to > > > > get the CPU's DT node further down the line. This causes issues with > > > > cpufreq-dt, as it triggers warnings when not finding a suitable DT node > > > > on CPU0. > > > > > > > > Change the way we choose whether to get MPIDR or not. Instead of > > > > depending on SMP check the number of CPUs defined in DT. Anything > 1 > > > > means MPIDR will be available. > > > > > > Except if someone accidentally wrote their Device Tree such as to have > > > > 1 CPU nodes, yet the CPU is not MP capable and reading the MPIDR > > > register does return the expected value, but that is wrong anyway. > > > > An UP device will most likely not have a MPIDR. That said I'm not sure this > > is > > always true. As per ARM1176JZ's TRM[1], the RPi1 CPU, if one was to get the > > MPIDR it would raise an undefined exception. > > > > The way I see it's an acceptable outcome as the DT is clearly wrong. > > It is, although you probably want to use of_get_available_child_count() > instead of of_get_child_count() since we could imagine that a boot > loader or some other boot program mangling the DT could intentionally > put a 'status = "disabled"' property on the non-boot CPU node for > whatever reason.
Good point, I'll fix it on v2.
signature.asc
Description: This is a digitally signed message part
