| Message ID | 20181121141204.22819-1-tiny.windzz@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | soc/tegra: refactor soc_is_tegra() | expand |
On 21/11/2018 14:12, Yangtao Li wrote: > of_find_node_by_path() acquires a reference to the node returned by > it and that reference needs to be dropped by its caller.soc_is_tegra() > doesn't do that, so fix it.Call of_machine_is_compatible() to refactor > soc_is_tegra() whcih automatically manages the reference count. > > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com> > --- > drivers/soc/tegra/common.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/drivers/soc/tegra/common.c b/drivers/soc/tegra/common.c > index cd8f41351add..0b40700b672a 100644 > --- a/drivers/soc/tegra/common.c > +++ b/drivers/soc/tegra/common.c > @@ -22,11 +22,13 @@ static const struct of_device_id tegra_machine_match[] = { > > bool soc_is_tegra(void) > { > - struct device_node *root; > + struct of_device_id *match = tegra_machine_match; > > - root = of_find_node_by_path("/"); > - if (!root) > - return false; > + while(match->compatible){ > + if(of_machine_is_compatible(match->compatible)) > + return true; > + match++; > + } > > - return of_match_node(tegra_machine_match, root) != NULL; > + return false; > } Ugh ... sorry, I thought that of_machine_is_compatible() looped through the matches. OK, let's stick with your initial fix. Cheers Jon
On Wed, Nov 21, 2018 at 02:34:57PM +0000, Jon Hunter wrote: > > On 21/11/2018 14:12, Yangtao Li wrote: > > of_find_node_by_path() acquires a reference to the node returned by > > it and that reference needs to be dropped by its caller.soc_is_tegra() > > doesn't do that, so fix it.Call of_machine_is_compatible() to refactor > > soc_is_tegra() whcih automatically manages the reference count. > > > > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com> > > --- > > drivers/soc/tegra/common.c | 12 +++++++----- > > 1 file changed, 7 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/soc/tegra/common.c b/drivers/soc/tegra/common.c > > index cd8f41351add..0b40700b672a 100644 > > --- a/drivers/soc/tegra/common.c > > +++ b/drivers/soc/tegra/common.c > > @@ -22,11 +22,13 @@ static const struct of_device_id tegra_machine_match[] = { > > > > bool soc_is_tegra(void) > > { > > - struct device_node *root; > > + struct of_device_id *match = tegra_machine_match; > > > > - root = of_find_node_by_path("/"); > > - if (!root) > > - return false; > > + while(match->compatible){ > > + if(of_machine_is_compatible(match->compatible)) > > + return true; > > + match++; > > + } > > > > - return of_match_node(tegra_machine_match, root) != NULL; > > + return false; > > } > > Ugh ... sorry, I thought that of_machine_is_compatible() looped through > the matches. OK, let's stick with your initial fix. Actually I prefer this one. Even if it is slightly more verbose, I think it's much clearer what's actually going on. Also this hides all of the OF node reference counting in a core function, so it's worth the extra line, in my opinion. Thierry
On Wed, Nov 21, 2018 at 10:43 PM Thierry Reding <thierry.reding@gmail.com> wrote: > > On Wed, Nov 21, 2018 at 02:34:57PM +0000, Jon Hunter wrote: > > > > On 21/11/2018 14:12, Yangtao Li wrote: > > > of_find_node_by_path() acquires a reference to the node returned by > > > it and that reference needs to be dropped by its caller.soc_is_tegra() > > > doesn't do that, so fix it.Call of_machine_is_compatible() to refactor > > > soc_is_tegra() whcih automatically manages the reference count. > > > > > > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com> > > > --- > > > drivers/soc/tegra/common.c | 12 +++++++----- > > > 1 file changed, 7 insertions(+), 5 deletions(-) > > > > > > diff --git a/drivers/soc/tegra/common.c b/drivers/soc/tegra/common.c > > > index cd8f41351add..0b40700b672a 100644 > > > --- a/drivers/soc/tegra/common.c > > > +++ b/drivers/soc/tegra/common.c > > > @@ -22,11 +22,13 @@ static const struct of_device_id tegra_machine_match[] = { > > > > > > bool soc_is_tegra(void) > > > { > > > - struct device_node *root; > > > + struct of_device_id *match = tegra_machine_match; > > > > > > - root = of_find_node_by_path("/"); > > > - if (!root) > > > - return false; > > > + while(match->compatible){ > > > + if(of_machine_is_compatible(match->compatible)) > > > + return true; > > > + match++; > > > + } > > > > > > - return of_match_node(tegra_machine_match, root) != NULL; > > > + return false; > > > } > > > > Ugh ... sorry, I thought that of_machine_is_compatible() looped through > > the matches. OK, let's stick with your initial fix. > > Actually I prefer this one. Even if it is slightly more verbose, I think > it's much clearer what's actually going on. Also this hides all of the > OF node reference counting in a core function, so it's worth the extra > line, in my opinion. > > Thierry Hi Jon： I like both, how aboout you? Yours， Yangtao
On 21/11/2018 14:47, Frank Lee wrote: > On Wed, Nov 21, 2018 at 10:43 PM Thierry Reding > <thierry.reding@gmail.com> wrote: >> >> On Wed, Nov 21, 2018 at 02:34:57PM +0000, Jon Hunter wrote: >>> >>> On 21/11/2018 14:12, Yangtao Li wrote: >>>> of_find_node_by_path() acquires a reference to the node returned by >>>> it and that reference needs to be dropped by its caller.soc_is_tegra() >>>> doesn't do that, so fix it.Call of_machine_is_compatible() to refactor >>>> soc_is_tegra() whcih automatically manages the reference count. >>>> >>>> Signed-off-by: Yangtao Li <tiny.windzz@gmail.com> >>>> --- >>>> drivers/soc/tegra/common.c | 12 +++++++----- >>>> 1 file changed, 7 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/drivers/soc/tegra/common.c b/drivers/soc/tegra/common.c >>>> index cd8f41351add..0b40700b672a 100644 >>>> --- a/drivers/soc/tegra/common.c >>>> +++ b/drivers/soc/tegra/common.c >>>> @@ -22,11 +22,13 @@ static const struct of_device_id tegra_machine_match[] = { >>>> >>>> bool soc_is_tegra(void) >>>> { >>>> - struct device_node *root; >>>> + struct of_device_id *match = tegra_machine_match; >>>> >>>> - root = of_find_node_by_path("/"); >>>> - if (!root) >>>> - return false; >>>> + while(match->compatible){ >>>> + if(of_machine_is_compatible(match->compatible)) >>>> + return true; >>>> + match++; >>>> + } >>>> >>>> - return of_match_node(tegra_machine_match, root) != NULL; >>>> + return false; >>>> } >>> >>> Ugh ... sorry, I thought that of_machine_is_compatible() looped through >>> the matches. OK, let's stick with your initial fix. >> >> Actually I prefer this one. Even if it is slightly more verbose, I think >> it's much clearer what's actually going on. Also this hides all of the >> OF node reference counting in a core function, so it's worth the extra >> line, in my opinion. >> >> Thierry > Hi Jon： > > I like both, how aboout you? Yes fine with me too. However, looks like there is some formatting that needs to be fixed up above. Please make sure you run checkpatch.pl on your patches. Otherwise ... Acked-by: Jon Hunter <jonathanh@nvidia.com> Thierry, pick up this version if you prefer. Cheers Jon
On Wed, Nov 21, 2018 at 02:50:54PM +0000, Jon Hunter wrote: > > On 21/11/2018 14:47, Frank Lee wrote: > > On Wed, Nov 21, 2018 at 10:43 PM Thierry Reding > > <thierry.reding@gmail.com> wrote: > >> > >> On Wed, Nov 21, 2018 at 02:34:57PM +0000, Jon Hunter wrote: > >>> > >>> On 21/11/2018 14:12, Yangtao Li wrote: > >>>> of_find_node_by_path() acquires a reference to the node returned by > >>>> it and that reference needs to be dropped by its caller.soc_is_tegra() > >>>> doesn't do that, so fix it.Call of_machine_is_compatible() to refactor > >>>> soc_is_tegra() whcih automatically manages the reference count. > >>>> > >>>> Signed-off-by: Yangtao Li <tiny.windzz@gmail.com> > >>>> --- > >>>> drivers/soc/tegra/common.c | 12 +++++++----- > >>>> 1 file changed, 7 insertions(+), 5 deletions(-) > >>>> > >>>> diff --git a/drivers/soc/tegra/common.c b/drivers/soc/tegra/common.c > >>>> index cd8f41351add..0b40700b672a 100644 > >>>> --- a/drivers/soc/tegra/common.c > >>>> +++ b/drivers/soc/tegra/common.c > >>>> @@ -22,11 +22,13 @@ static const struct of_device_id tegra_machine_match[] = { > >>>> > >>>> bool soc_is_tegra(void) > >>>> { > >>>> - struct device_node *root; > >>>> + struct of_device_id *match = tegra_machine_match; > >>>> > >>>> - root = of_find_node_by_path("/"); > >>>> - if (!root) > >>>> - return false; > >>>> + while(match->compatible){ > >>>> + if(of_machine_is_compatible(match->compatible)) > >>>> + return true; > >>>> + match++; > >>>> + } > >>>> > >>>> - return of_match_node(tegra_machine_match, root) != NULL; > >>>> + return false; > >>>> } > >>> > >>> Ugh ... sorry, I thought that of_machine_is_compatible() looped through > >>> the matches. OK, let's stick with your initial fix. > >> > >> Actually I prefer this one. Even if it is slightly more verbose, I think > >> it's much clearer what's actually going on. Also this hides all of the > >> OF node reference counting in a core function, so it's worth the extra > >> line, in my opinion. > >> > >> Thierry > > Hi Jon： > > > > I like both, how aboout you? > > Yes fine with me too. However, looks like there is some formatting that > needs to be fixed up above. Please make sure you run checkpatch.pl on > your patches. Otherwise ... > > Acked-by: Jon Hunter <jonathanh@nvidia.com> > > Thierry, pick up this version if you prefer. Yeah, I noticed the formatting issues, but I can take care of them while applying. Thanks, Thierry
On Wed, Nov 21, 2018 at 10:56 PM Thierry Reding <thierry.reding@gmail.com> wrote: > > On Wed, Nov 21, 2018 at 02:50:54PM +0000, Jon Hunter wrote: > > > > On 21/11/2018 14:47, Frank Lee wrote: > > > On Wed, Nov 21, 2018 at 10:43 PM Thierry Reding > > > <thierry.reding@gmail.com> wrote: > > >> > > >> On Wed, Nov 21, 2018 at 02:34:57PM +0000, Jon Hunter wrote: > > >>> > > >>> On 21/11/2018 14:12, Yangtao Li wrote: > > >>>> of_find_node_by_path() acquires a reference to the node returned by > > >>>> it and that reference needs to be dropped by its caller.soc_is_tegra() > > >>>> doesn't do that, so fix it.Call of_machine_is_compatible() to refactor > > >>>> soc_is_tegra() whcih automatically manages the reference count. > > >>>> > > >>>> Signed-off-by: Yangtao Li <tiny.windzz@gmail.com> > > >>>> --- > > >>>> drivers/soc/tegra/common.c | 12 +++++++----- > > >>>> 1 file changed, 7 insertions(+), 5 deletions(-) > > >>>> > > >>>> diff --git a/drivers/soc/tegra/common.c b/drivers/soc/tegra/common.c > > >>>> index cd8f41351add..0b40700b672a 100644 > > >>>> --- a/drivers/soc/tegra/common.c > > >>>> +++ b/drivers/soc/tegra/common.c > > >>>> @@ -22,11 +22,13 @@ static const struct of_device_id tegra_machine_match[] = { > > >>>> > > >>>> bool soc_is_tegra(void) > > >>>> { > > >>>> - struct device_node *root; > > >>>> + struct of_device_id *match = tegra_machine_match; > > >>>> > > >>>> - root = of_find_node_by_path("/"); > > >>>> - if (!root) > > >>>> - return false; > > >>>> + while(match->compatible){ > > >>>> + if(of_machine_is_compatible(match->compatible)) > > >>>> + return true; > > >>>> + match++; > > >>>> + } > > >>>> > > >>>> - return of_match_node(tegra_machine_match, root) != NULL; > > >>>> + return false; > > >>>> } > > >>> > > >>> Ugh ... sorry, I thought that of_machine_is_compatible() looped through > > >>> the matches. OK, let's stick with your initial fix. > > >> > > >> Actually I prefer this one. Even if it is slightly more verbose, I think > > >> it's much clearer what's actually going on. Also this hides all of the > > >> OF node reference counting in a core function, so it's worth the extra > > >> line, in my opinion. > > >> > > >> Thierry > > > Hi Jon： > > > > > > I like both, how aboout you? > > > > Yes fine with me too. However, looks like there is some formatting that > > needs to be fixed up above. Please make sure you run checkpatch.pl on > > your patches. Otherwise ... > > > > Acked-by: Jon Hunter <jonathanh@nvidia.com> > > > > Thierry, pick up this version if you prefer. > > Yeah, I noticed the formatting issues, but I can take care of them while > applying. > > Thanks, > Thierry Thierry， Thanks. :-) Yours, Yangtao
On Wed, Nov 21, 2018 at 09:12:04AM -0500, Yangtao Li wrote: > of_find_node_by_path() acquires a reference to the node returned by > it and that reference needs to be dropped by its caller.soc_is_tegra() > doesn't do that, so fix it.Call of_machine_is_compatible() to refactor > soc_is_tegra() whcih automatically manages the reference count. > > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com> > --- > drivers/soc/tegra/common.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) Applied to for-4.21/soc, thanks. Thierry
On Wed, Nov 21, 2018 at 3:40 PM Yangtao Li <tiny.windzz@gmail.com> wrote: > > of_find_node_by_path() acquires a reference to the node returned by > it and that reference needs to be dropped by its caller.soc_is_tegra() > doesn't do that, so fix it.Call of_machine_is_compatible() to refactor > soc_is_tegra() whcih automatically manages the reference count. > > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com> > --- > drivers/soc/tegra/common.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/drivers/soc/tegra/common.c b/drivers/soc/tegra/common.c > index cd8f41351add..0b40700b672a 100644 > --- a/drivers/soc/tegra/common.c > +++ b/drivers/soc/tegra/common.c > @@ -22,11 +22,13 @@ static const struct of_device_id tegra_machine_match[] = { > > bool soc_is_tegra(void) > { > - struct device_node *root; > + struct of_device_id *match = tegra_machine_match; > > - root = of_find_node_by_path("/"); > - if (!root) > - return false; > + while(match->compatible){ > + if(of_machine_is_compatible(match->compatible)) > + return true; > + match++; > + } > > - return of_match_node(tegra_machine_match, root) != NULL; > + return false; > } > -- > 2.17.0 >
On Thu, Nov 22, 2018 at 11:45 AM Arnd Bergmann <arnd@arndb.de> wrote: > > On Wed, Nov 21, 2018 at 3:40 PM Yangtao Li <tiny.windzz@gmail.com> wrote: > > > > of_find_node_by_path() acquires a reference to the node returned by > > it and that reference needs to be dropped by its caller.soc_is_tegra() > > doesn't do that, so fix it.Call of_machine_is_compatible() to refactor > > soc_is_tegra() whcih automatically manages the reference count. > > > > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com> > > --- Anders ran into a crash after this patch, on a non-tegra qemu platform: [ 0.055907] ASID allocator initialised with 32768 entries [ 0.063381] rcu: Hierarchical SRCU implementation. [ 0.137238] Unable to handle kernel paging request at virtual address ffff00000935f0c0 [ 0.137274] Mem abort info: [ 0.137291] ESR = 0x96000007 [ 0.137320] Exception class = DABT (current EL), IL = 32 bits [ 0.137337] SET = 0, FnV = 0 [ 0.137352] EA = 0, S1PTW = 0 [ 0.137370] Data abort info: [ 0.137387] ISV = 0, ISS = 0x00000007 [ 0.137401] CM = 0, WnR = 0 [ 0.137456] swapper pgtable: 4k pages, 48-bit VAs, pgdp = (____ptrval____) [ 0.137479] [ffff00000935f0c0] pgd=00000000bdfff003, pud=00000000bdffe003, pmd=00000000bdffa003, pte=0000000000000000 [ 0.137644] Internal error: Oops: 96000007 [#1] PREEMPT SMP [ 0.137766] Modules linked in: [ 0.137950] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.20.0-rc3-next-20181122-00006-g38d8a1f80349-dirty #2 [ 0.137975] Hardware name: linux,dummy-virt (DT) [ 0.138071] pstate: 00000085 (nzcv daIf -PAN -UAO) [ 0.138110] pc : __of_device_is_compatible+0x30/0x138 [ 0.138132] lr : of_device_is_compatible+0x40/0x68 [ 0.138147] sp : ffff00000804bc80 [ 0.138167] x29: ffff00000804bc80 x28: 0000000000000000 [ 0.138207] x27: 0000000000000000 x26: 0000000000000000 [ 0.138224] x25: ffff80007dfed2a8 x24: ffff000009301000 [ 0.138239] x23: 0000000000000000 x22: 0000000000000000 [ 0.138254] x21: ffff00000935f0c0 x20: ffff00000935f0c0 [ 0.138269] x19: 0000000000000000 x18: 0000000000000400 [ 0.138284] x17: 0000000000000000 x16: 0000000000000000 [ 0.138298] x15: 0000000000000400 x14: 0000000000000400 [ 0.138313] x13: 0000000000000000 x12: 0000000000000020 [ 0.138327] x11: 0000000000000008 x10: 0101010101010101 [ 0.138357] x9 : 6862726efffefeff x8 : 7f7f7f7f7f7f7f7f [ 0.138373] x7 : fefefefefefeff2e x6 : 0080808080808080 [ 0.138387] x5 : 0000000000000002 x4 : 0000000000000001 [ 0.138402] x3 : 0000000000000000 x2 : 0000000000000000 [ 0.138416] x1 : ffff00000935f0c0 x0 : ffff80007dfed2a8 [ 0.138475] Process swapper/0 (pid: 1, stack limit = 0x(____ptrval____)) [ 0.138540] Call trace: [ 0.138607] __of_device_is_compatible+0x30/0x138 [ 0.138632] of_device_is_compatible+0x40/0x68 [ 0.138654] of_machine_is_compatible+0x34/0x68 [ 0.138672] soc_is_tegra+0x2c/0x40 [ 0.138689] tegra_flowctrl_init+0x28/0x108 [ 0.138706] do_one_initcall+0x5c/0x178 [ 0.138722] kernel_init_freeable+0xd0/0x240 [ 0.138741] kernel_init+0x10/0x108 [ 0.138755] ret_from_fork+0x10/0x18 [ 0.138913] Code: b4000861 f90013b5 aa0103f5 52800013 (39400021) [ 0.139229] ---[ end trace d4d0fc77e9b04fa6 ]--- [ 0.139448] note: swapper/0[1] exited with preempt_count 1 [ 0.140598] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b [ 0.140767] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]--- I'm not completely sure what's wrong with the patch, but I assume it never worked on non-tegra machines. > > diff --git a/drivers/soc/tegra/common.c b/drivers/soc/tegra/common.c > > index cd8f41351add..0b40700b672a 100644 > > --- a/drivers/soc/tegra/common.c > > +++ b/drivers/soc/tegra/common.c > > @@ -22,11 +22,13 @@ static const struct of_device_id tegra_machine_match[] = { > > > > bool soc_is_tegra(void) > > { > > - struct device_node *root; > > + struct of_device_id *match = tegra_machine_match; > > > > - root = of_find_node_by_path("/"); > > - if (!root) > > - return false; > > + while(match->compatible){ > > + if(of_machine_is_compatible(match->compatible)) > > + return true; > > + match++; > > + } > > > > - return of_match_node(tegra_machine_match, root) != NULL; > > + return false; I would also note that this is a rather inefficient way to check for a particular platform, as we to do the string search through the DT for each entry in the table now instead of doing it once. Arnd
On Thu, Nov 22, 2018 at 6:49 PM Arnd Bergmann <arnd@arndb.de> wrote: > > On Thu, Nov 22, 2018 at 11:45 AM Arnd Bergmann <arnd@arndb.de> wrote: > > > > On Wed, Nov 21, 2018 at 3:40 PM Yangtao Li <tiny.windzz@gmail.com> wrote: > > > > > > of_find_node_by_path() acquires a reference to the node returned by > > > it and that reference needs to be dropped by its caller.soc_is_tegra() > > > doesn't do that, so fix it.Call of_machine_is_compatible() to refactor > > > soc_is_tegra() whcih automatically manages the reference count. > > > > > > Signed-off-by: Yangtao Li <tiny.windzz@gmail.com> > > > --- > > Anders ran into a crash after this patch, on a non-tegra qemu platform: > > [ 0.055907] ASID allocator initialised with 32768 entries > [ 0.063381] rcu: Hierarchical SRCU implementation. > [ 0.137238] Unable to handle kernel paging request at virtual > address ffff00000935f0c0 > [ 0.137274] Mem abort info: > [ 0.137291] ESR = 0x96000007 > [ 0.137320] Exception class = DABT (current EL), IL = 32 bits > [ 0.137337] SET = 0, FnV = 0 > [ 0.137352] EA = 0, S1PTW = 0 > [ 0.137370] Data abort info: > [ 0.137387] ISV = 0, ISS = 0x00000007 > [ 0.137401] CM = 0, WnR = 0 > [ 0.137456] swapper pgtable: 4k pages, 48-bit VAs, pgdp = (____ptrval____) > [ 0.137479] [ffff00000935f0c0] pgd=00000000bdfff003, > pud=00000000bdffe003, pmd=00000000bdffa003, pte=0000000000000000 > [ 0.137644] Internal error: Oops: 96000007 [#1] PREEMPT SMP > [ 0.137766] Modules linked in: > [ 0.137950] CPU: 0 PID: 1 Comm: swapper/0 Not tainted > 4.20.0-rc3-next-20181122-00006-g38d8a1f80349-dirty #2 > [ 0.137975] Hardware name: linux,dummy-virt (DT) > [ 0.138071] pstate: 00000085 (nzcv daIf -PAN -UAO) > [ 0.138110] pc : __of_device_is_compatible+0x30/0x138 > [ 0.138132] lr : of_device_is_compatible+0x40/0x68 > [ 0.138147] sp : ffff00000804bc80 > [ 0.138167] x29: ffff00000804bc80 x28: 0000000000000000 > [ 0.138207] x27: 0000000000000000 x26: 0000000000000000 > [ 0.138224] x25: ffff80007dfed2a8 x24: ffff000009301000 > [ 0.138239] x23: 0000000000000000 x22: 0000000000000000 > [ 0.138254] x21: ffff00000935f0c0 x20: ffff00000935f0c0 > [ 0.138269] x19: 0000000000000000 x18: 0000000000000400 > [ 0.138284] x17: 0000000000000000 x16: 0000000000000000 > [ 0.138298] x15: 0000000000000400 x14: 0000000000000400 > [ 0.138313] x13: 0000000000000000 x12: 0000000000000020 > [ 0.138327] x11: 0000000000000008 x10: 0101010101010101 > [ 0.138357] x9 : 6862726efffefeff x8 : 7f7f7f7f7f7f7f7f > [ 0.138373] x7 : fefefefefefeff2e x6 : 0080808080808080 > [ 0.138387] x5 : 0000000000000002 x4 : 0000000000000001 > [ 0.138402] x3 : 0000000000000000 x2 : 0000000000000000 > [ 0.138416] x1 : ffff00000935f0c0 x0 : ffff80007dfed2a8 > [ 0.138475] Process swapper/0 (pid: 1, stack limit = 0x(____ptrval____)) > [ 0.138540] Call trace: > [ 0.138607] __of_device_is_compatible+0x30/0x138 > [ 0.138632] of_device_is_compatible+0x40/0x68 > [ 0.138654] of_machine_is_compatible+0x34/0x68 > [ 0.138672] soc_is_tegra+0x2c/0x40 > [ 0.138689] tegra_flowctrl_init+0x28/0x108 > [ 0.138706] do_one_initcall+0x5c/0x178 > [ 0.138722] kernel_init_freeable+0xd0/0x240 > [ 0.138741] kernel_init+0x10/0x108 > [ 0.138755] ret_from_fork+0x10/0x18 > [ 0.138913] Code: b4000861 f90013b5 aa0103f5 52800013 (39400021) > [ 0.139229] ---[ end trace d4d0fc77e9b04fa6 ]--- > [ 0.139448] note: swapper/0[1] exited with preempt_count 1 > [ 0.140598] Kernel panic - not syncing: Attempted to kill init! > exitcode=0x0000000b > [ 0.140767] ---[ end Kernel panic - not syncing: Attempted to kill > init! exitcode=0x0000000b ]--- > > I'm not completely sure what's wrong with the patch, but I assume > it never worked on non-tegra machines. > > > > diff --git a/drivers/soc/tegra/common.c b/drivers/soc/tegra/common.c > > > index cd8f41351add..0b40700b672a 100644 > > > --- a/drivers/soc/tegra/common.c > > > +++ b/drivers/soc/tegra/common.c > > > @@ -22,11 +22,13 @@ static const struct of_device_id tegra_machine_match[] = { > > > > > > bool soc_is_tegra(void) > > > { > > > - struct device_node *root; > > > + struct of_device_id *match = tegra_machine_match; > > > > > > - root = of_find_node_by_path("/"); > > > - if (!root) > > > - return false; > > > + while(match->compatible){ > > > + if(of_machine_is_compatible(match->compatible)) > > > + return true; > > > + match++; > > > + } > > > > > > - return of_match_node(tegra_machine_match, root) != NULL; > > > + return false; > > I would also note that this is a rather inefficient way to check > for a particular platform, as we to do the string search through > the DT for each entry in the table now instead of doing it once. > > Arnd Hi Jon: Do you think we should stick to the new way or go back to the way before? Yours, Yangtao
diff --git a/drivers/soc/tegra/common.c b/drivers/soc/tegra/common.c index cd8f41351add..0b40700b672a 100644 --- a/drivers/soc/tegra/common.c +++ b/drivers/soc/tegra/common.c @@ -22,11 +22,13 @@ static const struct of_device_id tegra_machine_match[] = { bool soc_is_tegra(void) { - struct device_node *root; + struct of_device_id *match = tegra_machine_match; - root = of_find_node_by_path("/"); - if (!root) - return false; + while(match->compatible){ + if(of_machine_is_compatible(match->compatible)) + return true; + match++; + } - return of_match_node(tegra_machine_match, root) != NULL; + return false; }
of_find_node_by_path() acquires a reference to the node returned by it and that reference needs to be dropped by its caller.soc_is_tegra() doesn't do that, so fix it.Call of_machine_is_compatible() to refactor soc_is_tegra() whcih automatically manages the reference count. Signed-off-by: Yangtao Li <tiny.windzz@gmail.com> --- drivers/soc/tegra/common.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)