On Wed, 19 Feb 2014 16:23:02 -0500, Paul Gortmaker <paul.gortmaker@xxxxxxxxxxxxx> wrote: > On 14-02-19 03:41 PM, Grant Likely wrote: > > On Wed, 19 Feb 2014 13:25:54 -0500, Paul Gortmaker <paul.gortmaker@xxxxxxxxxxxxx> wrote: > >> On Thu, Feb 13, 2014 at 2:01 PM, Rob Herring <robherring2@xxxxxxxxx> wrote: > >>> On Wed, Feb 12, 2014 at 5:38 AM, Kevin Hao <haokexin@xxxxxxxxx> wrote: > >>>> When the device node do have a compatible property, we definitely > >>>> prefer the compatible match besides the type and name. Only if > >>>> there is no such a match, we then consider the candidate which > >>>> doesn't have compatible entry but do match the type or name with > >>>> the device node. > >>>> > >>>> This is based on a patch from Sebastian Hesselbarth. > >>>> http://patchwork.ozlabs.org/patch/319434/ > >>>> > >>>> I did some code refactoring and also fixed a bug in the original patch. > >>> > >>> I'm inclined to just revert this once again and avoid possibly > >>> breaking yet another platform. > >> > >> Well, for what it is worth, today's (Feb19th) linux-next tree fails to boot > >> on my sbc8548. It fails with: > > > > I think I've got it fixed now with the latest series. Please try the > > devicetree/merge branch on git://git.secretlab.ca/git/linux > > That seems to work. Great, thanks for the testing. Can I add a Tested-by line for you? g. > > libphy: Freescale PowerQUICC MII Bus: probed > libphy: Freescale PowerQUICC MII Bus: probed > fsl-gianfar e0024000.ethernet: enabled errata workarounds, flags: 0x4 > fsl-gianfar e0024000.ethernet eth0: mac: 02:e0:0c:00:05:fd > fsl-gianfar e0024000.ethernet eth0: Running with NAPI enabled > fsl-gianfar e0024000.ethernet eth0: RX BD ring size for Q[0]: 256 > fsl-gianfar e0024000.ethernet eth0: TX BD ring size for Q[0]: 256 > fsl-gianfar e0025000.ethernet: enabled errata workarounds, flags: 0x4 > fsl-gianfar e0025000.ethernet eth1: mac: 02:e0:0c:00:06:fd > fsl-gianfar e0025000.ethernet eth1: Running with NAPI enabled > fsl-gianfar e0025000.ethernet eth1: RX BD ring size for Q[0]: 256 > fsl-gianfar e0025000.ethernet eth1: TX BD ring size for Q[0]: 256 > TCP: cubic registered > Initializing XFRM netlink socket > NET: Registered protocol family 17 > > ... > > root@sbc8548:~# cat /proc/version > Linux version 3.14.0-rc3-00024-g7119f42057c6 (paul@yow-lpgnfs-02) (gcc version 4.5.2 (GCC) ) #1 Wed Feb 19 16:08:17 EST 2014 > root@sbc8548:~# > > Paul. > -- > > > > > g. > > > >> ----------------------------------------------- > >> libphy: Freescale PowerQUICC MII Bus: probed > >> mdio_bus ethernet@e002400: /soc8548@e0000000/ethernet@24000/mdio@520 > >> PHY address 1312 is too large > >> libphy: Freescale PowerQUICC MII Bus: probed > >> libphy: Freescale PowerQUICC MII Bus: probed > >> mdio_bus ethernet@e002500: /soc8548@e0000000/ethernet@25000/mdio@520 > >> PHY address 1312 is too large > >> libphy: Freescale PowerQUICC MII Bus: probed > >> TCP: cubic registered > >> Initializing XFRM netlink socket > >> NET: Registered protocol family 17 > >> <fail nfs mount> > >> ----------------------------------------------- > >> > >> On a normal boot, we should see this: > >> ----------------------------------------------- > >> libphy: Freescale PowerQUICC MII Bus: probed > >> libphy: Freescale PowerQUICC MII Bus: probed > >> fsl-gianfar e0024000.ethernet: enabled errata workarounds, flags: 0x4 > >> fsl-gianfar e0024000.ethernet eth0: mac: 02:e0:0c:00:05:fd > >> fsl-gianfar e0024000.ethernet eth0: Running with NAPI enabled > >> fsl-gianfar e0024000.ethernet eth0: RX BD ring size for Q[0]: 256 > >> fsl-gianfar e0024000.ethernet eth0: TX BD ring size for Q[0]: 256 > >> fsl-gianfar e0025000.ethernet: enabled errata workarounds, flags: 0x4 > >> fsl-gianfar e0025000.ethernet eth1: mac: 02:e0:0c:00:06:fd > >> fsl-gianfar e0025000.ethernet eth1: Running with NAPI enabled > >> fsl-gianfar e0025000.ethernet eth1: RX BD ring size for Q[0]: 256 > >> fsl-gianfar e0025000.ethernet eth1: TX BD ring size for Q[0]: 256 > >> TCP: cubic registered > >> Initializing XFRM netlink socket > >> NET: Registered protocol family 17 > >> ----------------------------------------------- > >> > >> > >> Git bisect says: > >> > >> ee8b26ad943aa34acc03ae6cde2b81d8d3d483d4 is the first bad commit > >> commit ee8b26ad943aa34acc03ae6cde2b81d8d3d483d4 > >> Author: Kevin Hao <haokexin@xxxxxxxxx> > >> Date: Tue Feb 18 15:57:30 2014 +0800 > >> > >> of: reimplement the matching method for __of_match_node() > >> > >> In the current implementation of __of_match_node(), it will compare > >> each given match entry against all the node's compatible strings > >> with of_device_is_compatible(). > >> > >> To achieve multiple compatible strings per node with ordering from > >> specific to generic, this requires given matches to be ordered from > >> specific to generic. For most of the drivers this is not true and > >> also an alphabetical ordering is more sane there. > >> > >> Therefore, we define a following priority order for the match, and > >> then scan all the entries to find the best match. > >> 1. specific compatible && type && name > >> 2. specific compatible && type > >> 3. specific compatible && name > >> 4. specific compatible > >> 5. general compatible && type && name > >> 6. general compatible && type > >> 7. general compatible && name > >> 8. general compatible > >> 9. type && name > >> 10. type > >> 11. name > >> > >> This is based on some pseudo-codes provided by Grant Likely. > >> > >> Signed-off-by: Kevin Hao <haokexin@xxxxxxxxx> > >> [grant.likely: Changed multiplier to 4 which makes more sense] > >> Signed-off-by: Grant Likely <grant.likely@xxxxxxxxxx> > >> > >> :040000 040000 8f5dd19174417aece63b308ff299a5dbe2efa5a0 > >> 8401b0e3903e23e973845ee75b26b04345d803d2 M drivers > >> > >> As a double check, I checked out the head of linux-next, and did a > >> revert of the above commit, and my sbc8548 can then boot properly. > >> > >> Not doing anything fancy ; using the defconfig exactly as-is, and > >> ensuring the dtb is fresh from linux-next HEAD of today. > >> > >> Thanks, > >> Paul. > >> -- > >> > >>> > >>> However, I think I would like to see this structured differently. We > >>> basically have 2 ways of matching: the existing pre-3.14 way and the > >>> desired match on best compatible only. All new bindings should match > >>> with the new way and the old way needs to be kept for compatibility. > >>> So lets structure the code that way. Search the match table first for > >>> best compatible with name and type NULL, then search the table the old > >>> way. I realize it appears you are doing this, but it is not clear this > >>> is the intent of the code. I would like to see this written as a patch > >>> with commit 105353145eafb3ea919 reverted first and you add a new match > >>> function to call first and then fallback to the existing function. > >>> > >>> Rob > >>> > >>>> > >>>> Cc: Sebastian Hesselbarth <sebastian.hesselbarth@xxxxxxxxx> > >>>> Signed-off-by: Kevin Hao <haokexin@xxxxxxxxx> > >>>> --- > >>>> drivers/of/base.c | 55 +++++++++++++++++++++++++++++++++++++------------------ > >>>> 1 file changed, 37 insertions(+), 18 deletions(-) > >>>> > >>>> diff --git a/drivers/of/base.c b/drivers/of/base.c > >>>> index ff85450d5683..9d655df458bd 100644 > >>>> --- a/drivers/of/base.c > >>>> +++ b/drivers/of/base.c > >>>> @@ -730,32 +730,45 @@ out: > >>>> } > >>>> EXPORT_SYMBOL(of_find_node_with_property); > >>>> > >>>> +static int of_match_type_or_name(const struct device_node *node, > >>>> + const struct of_device_id *m) > >>>> +{ > >>>> + int match = 1; > >>>> + > >>>> + if (m->name[0]) > >>>> + match &= node->name && !strcmp(m->name, node->name); > >>>> + > >>>> + if (m->type[0]) > >>>> + match &= node->type && !strcmp(m->type, node->type); > >>>> + > >>>> + return match; > >>>> +} > >>>> + > >>>> static > >>>> const struct of_device_id *__of_match_node(const struct of_device_id *matches, > >>>> const struct device_node *node) > >>>> { > >>>> const char *cp; > >>>> int cplen, l; > >>>> + const struct of_device_id *m; > >>>> + int match; > >>>> > >>>> if (!matches) > >>>> return NULL; > >>>> > >>>> cp = __of_get_property(node, "compatible", &cplen); > >>>> - do { > >>>> - const struct of_device_id *m = matches; > >>>> + while (cp && (cplen > 0)) { > >>>> + m = matches; > >>>> > >>>> /* Check against matches with current compatible string */ > >>>> while (m->name[0] || m->type[0] || m->compatible[0]) { > >>>> - int match = 1; > >>>> - if (m->name[0]) > >>>> - match &= node->name > >>>> - && !strcmp(m->name, node->name); > >>>> - if (m->type[0]) > >>>> - match &= node->type > >>>> - && !strcmp(m->type, node->type); > >>>> - if (m->compatible[0]) > >>>> - match &= cp > >>>> - && !of_compat_cmp(m->compatible, cp, > >>>> + if (!m->compatible[0]) { > >>>> + m++; > >>>> + continue; > >>>> + } > >>>> + > >>>> + match = of_match_type_or_name(node, m); > >>>> + match &= cp && !of_compat_cmp(m->compatible, cp, > >>>> strlen(m->compatible)); > >>>> if (match) > >>>> return m; > >>>> @@ -763,12 +776,18 @@ const struct of_device_id *__of_match_node(const struct of_device_id *matches, > >>>> } > >>>> > >>>> /* Get node's next compatible string */ > >>>> - if (cp) { > >>>> - l = strlen(cp) + 1; > >>>> - cp += l; > >>>> - cplen -= l; > >>>> - } > >>>> - } while (cp && (cplen > 0)); > >>>> + l = strlen(cp) + 1; > >>>> + cp += l; > >>>> + cplen -= l; > >>>> + } > >>>> + > >>>> + m = matches; > >>>> + /* Check against matches without compatible string */ > >>>> + while (m->name[0] || m->type[0] || m->compatible[0]) { > >>>> + if (!m->compatible[0] && of_match_type_or_name(node, m)) > >>>> + return m; > >>>> + m++; > >>>> + } > >>>> > >>>> return NULL; > >>>> } > >>>> -- > >>>> 1.8.5.3 > >>>> > >>>> -- > >>>> To unsubscribe from this list: send the line "unsubscribe devicetree" in > >>>> the body of a message to majordomo@xxxxxxxxxxxxxxx > >>>> More majordomo info at http://vger.kernel.org/majordomo-info.html > >>> _______________________________________________ > >>> Linuxppc-dev mailing list > >>> Linuxppc-dev@xxxxxxxxxxxxxxxx > >>> https://lists.ozlabs.org/listinfo/linuxppc-dev > > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
