The function of_match_device() should tell if a struct device matches an
of_device_id list and return the specific entry of that table matches the
device best.
The underlying __of_match_node() implements the wrong search algorithm. It
iterates over the list of of_device_ids, comparing the first compatible with
_all_ compatibles of the struct device, then the second compatible of
of_device_id and so on.
This leads to a problem, if the device has more than one compatible that match
the of_device_id list. The implemented search algorithm may find not the "best"
match. As the compatible list in the device is sorted from most to least
specific.
For example:
The imx28.dtsi gives this compatible string for its CAN core:
> compatible = "fsl,imx28-flexcan", "fsl,p1010-flexcan";
The flexcan driver defines:
> static const struct of_device_id flexcan_of_match[] = {
> { .compatible = "fsl,p1010-flexcan", .data = &fsl_p1010_devtype_data, },
> { .compatible = "fsl,imx28-flexcan", .data = &fsl_imx28_devtype_data, },
> { .compatible = "fsl,imx6q-flexcan", .data = &fsl_imx6q_devtype_data, },
> { /* sentinel */ },
> };
The "p1010" was the first Freescale SoC with the flexcan core. But this SoC has
a bug, so a workaround has to be enabled in the driver. The mx28 has this bug
fixed, so we don't need this quite costly workaround.
The __of_match_node() will compare:
from of_device_id from device
fsl,p1010-flexcan fsl,imx28-flexcan
fsl,p1010-flexcan fsl,p1010-flexcan -> MATCH
The of_match_device() function as it currently is implemented will always
return p1010 not the mx28.
This patch fixes the problem by exchanging outer and inner loop. The first
compatible of a device is compared against all compatible from the of_device_id
list, then the second device compatible and so on.
Signed-off-by: Marc Kleine-Budde <[email protected]>
---
drivers/of/base.c | 40 +++++++++++++++++++++++++++++++++++-----
1 file changed, 35 insertions(+), 5 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 865d3f6..5bff2fe 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -349,6 +349,35 @@ static int __of_device_is_compatible(const struct device_node *device,
return 0;
}
+/** Returns the specific of_device_id, if the it matches one of the
+ * strings in the device's "compatible" property
+ */
+static
+const struct of_device_id *__of_device_matches_compatible(const struct device_node *device,
+ const struct of_device_id *matches)
+{
+ const struct of_device_id *m;
+ const char* cp;
+ int cplen, l;
+
+ cp = __of_get_property(device, "compatible", &cplen);
+ if (cp == NULL)
+ return NULL;
+ while (cplen > 0) {
+ m = matches;
+ while (m->compatible[0]) {
+ if (of_compat_cmp(cp, m->compatible, strlen(m->compatible)) == 0)
+ return m;
+ m++;
+ }
+ l = strlen(cp) + 1;
+ cp += l;
+ cplen -= l;
+ }
+
+ return NULL;
+}
+
/** Checks if the given "compat" string matches one of the strings in
* the device's "compatible" property
*/
@@ -717,22 +746,23 @@ const struct of_device_id *__of_match_node(const struct of_device_id *matches,
{
if (!matches)
return NULL;
-
- while (matches->name[0] || matches->type[0] || matches->compatible[0]) {
+ while (matches->name[0] || matches->type[0]) {
int match = 1;
+
if (matches->name[0])
match &= node->name
&& !strcmp(matches->name, node->name);
if (matches->type[0])
match &= node->type
&& !strcmp(matches->type, node->type);
- if (matches->compatible[0])
- match &= __of_device_is_compatible(node,
- matches->compatible);
if (match)
return matches;
matches++;
}
+
+ if (matches->compatible[0])
+ return __of_device_matches_compatible(node, matches);
+
return NULL;
}
--
1.8.4.rc3
On 10/03/2013 01:51 PM, Marc Kleine-Budde wrote:
> The function of_match_device() should tell if a struct device matches an
> of_device_id list and return the specific entry of that table matches the
> device best.
>
> The underlying __of_match_node() implements the wrong search algorithm. It
> iterates over the list of of_device_ids, comparing the first compatible with
> _all_ compatibles of the struct device, then the second compatible of
> of_device_id and so on.
>
> This leads to a problem, if the device has more than one compatible that match
> the of_device_id list. The implemented search algorithm may find not the "best"
> match. As the compatible list in the device is sorted from most to least
> specific.
>
> For example:
>
> The imx28.dtsi gives this compatible string for its CAN core:
>
>> compatible = "fsl,imx28-flexcan", "fsl,p1010-flexcan";
>
> The flexcan driver defines:
>
>> static const struct of_device_id flexcan_of_match[] = {
>> { .compatible = "fsl,p1010-flexcan", .data = &fsl_p1010_devtype_data, },
>> { .compatible = "fsl,imx28-flexcan", .data = &fsl_imx28_devtype_data, },
>> { .compatible = "fsl,imx6q-flexcan", .data = &fsl_imx6q_devtype_data, },
>> { /* sentinel */ },
>> };
>
> The "p1010" was the first Freescale SoC with the flexcan core. But this SoC has
> a bug, so a workaround has to be enabled in the driver. The mx28 has this bug
> fixed, so we don't need this quite costly workaround.
>
> The __of_match_node() will compare:
>
> from of_device_id from device
> fsl,p1010-flexcan fsl,imx28-flexcan
> fsl,p1010-flexcan fsl,p1010-flexcan -> MATCH
>
> The of_match_device() function as it currently is implemented will always
> return p1010 not the mx28.
>
> This patch fixes the problem by exchanging outer and inner loop. The first
> compatible of a device is compared against all compatible from the of_device_id
> list, then the second device compatible and so on.
This has been an issue for some time. A fix has been attempted before
and reverted if you look at the git history:
commit bc51b0c22cebf5c311a6f1895fcca9f78efd0478
Author: Linus Torvalds <[email protected]>
Date: Tue Jul 10 12:49:32 2012 -0700
Revert "of: match by compatible property first"
This reverts commit 107a84e61cdd3406c842a0e4be7efffd3a05dba6.
Meelis Roos reports a regression since 3.5-rc5 that stops Sun Fire V100
and Sun Netra X1 sparc64 machines from booting, hanging after enabling
serial console. He bisected it to commit 107a84e61cdd.
Rob Herring explains:
"The problem is match combinations of compatible plus name and/or type
fail to match correctly. I have a fix for this, but given how late it
is for 3.5 I think it is best to revert this for now. There could be
other cases that rely on the current although wrong behavior. I will
post an updated version for 3.6."
Bisected-and-reported-by: Meelis Roos <[email protected]>
Requested-by: Rob Herring <[email protected]>
Cc: Thierry Reding <[email protected]>
Cc: Grant Likely <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
There was also a fix attempted for this and the discussion here:
http://www.mail-archive.com/[email protected]/msg60163.html
You patch would hit the same issues I believe.
Rob
>
> Signed-off-by: Marc Kleine-Budde <[email protected]>
> ---
> drivers/of/base.c | 40 +++++++++++++++++++++++++++++++++++-----
> 1 file changed, 35 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 865d3f6..5bff2fe 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -349,6 +349,35 @@ static int __of_device_is_compatible(const struct device_node *device,
> return 0;
> }
>
> +/** Returns the specific of_device_id, if the it matches one of the
> + * strings in the device's "compatible" property
> + */
> +static
> +const struct of_device_id *__of_device_matches_compatible(const struct device_node *device,
> + const struct of_device_id *matches)
> +{
> + const struct of_device_id *m;
> + const char* cp;
> + int cplen, l;
> +
> + cp = __of_get_property(device, "compatible", &cplen);
> + if (cp == NULL)
> + return NULL;
> + while (cplen > 0) {
> + m = matches;
> + while (m->compatible[0]) {
> + if (of_compat_cmp(cp, m->compatible, strlen(m->compatible)) == 0)
> + return m;
> + m++;
> + }
> + l = strlen(cp) + 1;
> + cp += l;
> + cplen -= l;
> + }
> +
> + return NULL;
> +}
> +
> /** Checks if the given "compat" string matches one of the strings in
> * the device's "compatible" property
> */
> @@ -717,22 +746,23 @@ const struct of_device_id *__of_match_node(const struct of_device_id *matches,
> {
> if (!matches)
> return NULL;
> -
> - while (matches->name[0] || matches->type[0] || matches->compatible[0]) {
> + while (matches->name[0] || matches->type[0]) {
> int match = 1;
> +
> if (matches->name[0])
> match &= node->name
> && !strcmp(matches->name, node->name);
> if (matches->type[0])
> match &= node->type
> && !strcmp(matches->type, node->type);
> - if (matches->compatible[0])
> - match &= __of_device_is_compatible(node,
> - matches->compatible);
> if (match)
> return matches;
> matches++;
> }
> +
> + if (matches->compatible[0])
> + return __of_device_matches_compatible(node, matches);
> +
> return NULL;
> }
>
>
On 10/03/2013 10:37 PM, Rob Herring wrote:
> On 10/03/2013 01:51 PM, Marc Kleine-Budde wrote:
>> The function of_match_device() should tell if a struct device matches an
>> of_device_id list and return the specific entry of that table matches the
>> device best.
>>
>> The underlying __of_match_node() implements the wrong search algorithm. It
>> iterates over the list of of_device_ids, comparing the first compatible with
>> _all_ compatibles of the struct device, then the second compatible of
>> of_device_id and so on.
>>
>> This leads to a problem, if the device has more than one compatible that match
>> the of_device_id list. The implemented search algorithm may find not the "best"
>> match. As the compatible list in the device is sorted from most to least
>> specific.
>>
>> For example:
>>
>> The imx28.dtsi gives this compatible string for its CAN core:
>>
>>> compatible = "fsl,imx28-flexcan", "fsl,p1010-flexcan";
>>
>> The flexcan driver defines:
>>
>>> static const struct of_device_id flexcan_of_match[] = {
>>> { .compatible = "fsl,p1010-flexcan", .data = &fsl_p1010_devtype_data, },
>>> { .compatible = "fsl,imx28-flexcan", .data = &fsl_imx28_devtype_data, },
>>> { .compatible = "fsl,imx6q-flexcan", .data = &fsl_imx6q_devtype_data, },
>>> { /* sentinel */ },
>>> };
>>
>> The "p1010" was the first Freescale SoC with the flexcan core. But this SoC has
>> a bug, so a workaround has to be enabled in the driver. The mx28 has this bug
>> fixed, so we don't need this quite costly workaround.
>>
>> The __of_match_node() will compare:
>>
>> from of_device_id from device
>> fsl,p1010-flexcan fsl,imx28-flexcan
>> fsl,p1010-flexcan fsl,p1010-flexcan -> MATCH
>>
>> The of_match_device() function as it currently is implemented will always
>> return p1010 not the mx28.
>>
>> This patch fixes the problem by exchanging outer and inner loop. The first
>> compatible of a device is compared against all compatible from the of_device_id
>> list, then the second device compatible and so on.
>
> This has been an issue for some time. A fix has been attempted before
> and reverted if you look at the git history:
..should have done this before creating the patch.
> commit bc51b0c22cebf5c311a6f1895fcca9f78efd0478
> Author: Linus Torvalds <[email protected]>
> Date: Tue Jul 10 12:49:32 2012 -0700
>
> Revert "of: match by compatible property first"
>
> This reverts commit 107a84e61cdd3406c842a0e4be7efffd3a05dba6.
>
> Meelis Roos reports a regression since 3.5-rc5 that stops Sun Fire V100
> and Sun Netra X1 sparc64 machines from booting, hanging after enabling
> serial console. He bisected it to commit 107a84e61cdd.
>
> Rob Herring explains:
> "The problem is match combinations of compatible plus name and/or type
> fail to match correctly. I have a fix for this, but given how late it
> is for 3.5 I think it is best to revert this for now. There could be
> other cases that rely on the current although wrong behavior. I will
> post an updated version for 3.6."
>
> Bisected-and-reported-by: Meelis Roos <[email protected]>
> Requested-by: Rob Herring <[email protected]>
> Cc: Thierry Reding <[email protected]>
> Cc: Grant Likely <[email protected]>
> Signed-off-by: Linus Torvalds <[email protected]>
>
> There was also a fix attempted for this and the discussion here:
>
> http://www.mail-archive.com/[email protected]/msg60163.html
>
> You patch would hit the same issues I believe.
Yes probably, are the OF patterns from the failing sparcs available
somewhere, so that we can test a better implementation?
I'll rearrange the drivers instead. BTW: what about the patch you
mentioned in the above revert?
tnx,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
On 10/03/2013 04:51 PM, Marc Kleine-Budde wrote:
> On 10/03/2013 10:37 PM, Rob Herring wrote:
>> On 10/03/2013 01:51 PM, Marc Kleine-Budde wrote:
>>> The function of_match_device() should tell if a struct device
>>> matches an of_device_id list and return the specific entry of
>>> that table matches the device best.
>>>
>>> The underlying __of_match_node() implements the wrong search
>>> algorithm. It iterates over the list of of_device_ids,
>>> comparing the first compatible with _all_ compatibles of the
>>> struct device, then the second compatible of of_device_id and
>>> so on.
>>>
>>> This leads to a problem, if the device has more than one
>>> compatible that match the of_device_id list. The implemented
>>> search algorithm may find not the "best" match. As the
>>> compatible list in the device is sorted from most to least
>>> specific.
>>>
>>> For example:
>>>
>>> The imx28.dtsi gives this compatible string for its CAN core:
>>>
>>>> compatible = "fsl,imx28-flexcan", "fsl,p1010-flexcan";
>>>
>>> The flexcan driver defines:
>>>
>>>> static const struct of_device_id flexcan_of_match[] = { {
>>>> .compatible = "fsl,p1010-flexcan", .data =
>>>> &fsl_p1010_devtype_data, }, { .compatible =
>>>> "fsl,imx28-flexcan", .data = &fsl_imx28_devtype_data, }, {
>>>> .compatible = "fsl,imx6q-flexcan", .data =
>>>> &fsl_imx6q_devtype_data, }, { /* sentinel */ }, };
>>>
>>> The "p1010" was the first Freescale SoC with the flexcan core.
>>> But this SoC has a bug, so a workaround has to be enabled in
>>> the driver. The mx28 has this bug fixed, so we don't need this
>>> quite costly workaround.
>>>
>>> The __of_match_node() will compare:
>>>
>>> from of_device_id from device fsl,p1010-flexcan
>>> fsl,imx28-flexcan fsl,p1010-flexcan fsl,p1010-flexcan ->
>>> MATCH
>>>
>>> The of_match_device() function as it currently is implemented
>>> will always return p1010 not the mx28.
>>>
>>> This patch fixes the problem by exchanging outer and inner
>>> loop. The first compatible of a device is compared against all
>>> compatible from the of_device_id list, then the second device
>>> compatible and so on.
>>
>> This has been an issue for some time. A fix has been attempted
>> before and reverted if you look at the git history:
>
> ..should have done this before creating the patch.
>
>> commit bc51b0c22cebf5c311a6f1895fcca9f78efd0478 Author: Linus
>> Torvalds <[email protected]> Date: Tue Jul 10
>> 12:49:32 2012 -0700
>>
>> Revert "of: match by compatible property first"
>>
>> This reverts commit 107a84e61cdd3406c842a0e4be7efffd3a05dba6.
>>
>> Meelis Roos reports a regression since 3.5-rc5 that stops Sun
>> Fire V100 and Sun Netra X1 sparc64 machines from booting, hanging
>> after enabling serial console. He bisected it to commit
>> 107a84e61cdd.
>>
>> Rob Herring explains: "The problem is match combinations of
>> compatible plus name and/or type fail to match correctly. I have
>> a fix for this, but given how late it is for 3.5 I think it is
>> best to revert this for now. There could be other cases that
>> rely on the current although wrong behavior. I will post an
>> updated version for 3.6."
>>
>> Bisected-and-reported-by: Meelis Roos <[email protected]>
>> Requested-by: Rob Herring <[email protected]> Cc: Thierry
>> Reding <[email protected]> Cc: Grant Likely
>> <[email protected]> Signed-off-by: Linus Torvalds
>> <[email protected]>
>>
>> There was also a fix attempted for this and the discussion here:
>>
>> http://www.mail-archive.com/[email protected]/msg60163.html
>>
>>
>>
You patch would hit the same issues I believe.
>
> Yes probably, are the OF patterns from the failing sparcs
> available somewhere, so that we can test a better implementation?
Not that I'm aware of. It appeared to be the serial driver. It may be
evident looking at the sparc serial driver match table.
> I'll rearrange the drivers instead. BTW: what about the patch you
> mentioned in the above revert?
My position has been we should change the driver ordering, but others
have disagreed.
Scott had some issues with my patch and my assumptions may not be
right. Probably re-applying the original patch plus Scott's fix is the
right answer. We just need to review the combined change closely to
ensure we maintain current behavior where needed.
Rob
Hi Marc,
On Thu, Oct 3, 2013 at 3:51 PM, Marc Kleine-Budde <[email protected]> wrote:
> For example:
>
> The imx28.dtsi gives this compatible string for its CAN core:
>
>> compatible = "fsl,imx28-flexcan", "fsl,p1010-flexcan";
>
> The flexcan driver defines:
>
>> static const struct of_device_id flexcan_of_match[] = {
>> { .compatible = "fsl,p1010-flexcan", .data = &fsl_p1010_devtype_data, },
>> { .compatible = "fsl,imx28-flexcan", .data = &fsl_imx28_devtype_data, },
>> { .compatible = "fsl,imx6q-flexcan", .data = &fsl_imx6q_devtype_data, },
>> { /* sentinel */ },
>> };
>
> The "p1010" was the first Freescale SoC with the flexcan core. But this SoC has
> a bug, so a workaround has to be enabled in the driver. The mx28 has this bug
> fixed, so we don't need this quite costly workaround.
What about defining in imx28.dtsi:
compatible = "fsl,imx28-flexcan".
and the in the flexcan driver we could do the same as in the fec_main driver:
static struct platform_device_id fec_devtype[] = {
{
/* keep it for coldfire */
.name = DRIVER_NAME,
.driver_data = 0,
}, {
.name = "imx25-fec",
.driver_data = FEC_QUIRK_USE_GASKET,
}, {
.name = "imx27-fec",
.driver_data = 0,
}, {
.name = "imx28-fec",
.driver_data = FEC_QUIRK_ENET_MAC | FEC_QUIRK_SWAP_FRAME,
}, {
.name = "imx6q-fec",
.driver_data = FEC_QUIRK_ENET_MAC | FEC_QUIRK_HAS_GBIT |
FEC_QUIRK_HAS_BUFDESC_EX | FEC_QUIRK_HAS_CSUM |
FEC_QUIRK_HAS_VLAN | FEC_QUIRK_ERR006358,
}, {
.name = "mvf600-fec",
.driver_data = FEC_QUIRK_ENET_MAC,
}, {
/* sentinel */
}
};
So that we know which SoC needs to have the workaround applied or not.
On 10/05/2013 07:46 PM, Fabio Estevam wrote:
> Hi Marc,
>
> On Thu, Oct 3, 2013 at 3:51 PM, Marc Kleine-Budde <[email protected]> wrote:
>
>> For example:
>>
>> The imx28.dtsi gives this compatible string for its CAN core:
>>
>>> compatible = "fsl,imx28-flexcan", "fsl,p1010-flexcan";
>>
>> The flexcan driver defines:
>>
>>> static const struct of_device_id flexcan_of_match[] = {
>>> { .compatible = "fsl,p1010-flexcan", .data = &fsl_p1010_devtype_data, },
>>> { .compatible = "fsl,imx28-flexcan", .data = &fsl_imx28_devtype_data, },
>>> { .compatible = "fsl,imx6q-flexcan", .data = &fsl_imx6q_devtype_data, },
>>> { /* sentinel */ },
>>> };
>>
>> The "p1010" was the first Freescale SoC with the flexcan core. But this SoC has
>> a bug, so a workaround has to be enabled in the driver. The mx28 has this bug
>> fixed, so we don't need this quite costly workaround.
>
> What about defining in imx28.dtsi:
> compatible = "fsl,imx28-flexcan".
It already works with changing only the driver.
> and the in the flexcan driver we could do the same as in the fec_main driver:
>
> static struct platform_device_id fec_devtype[] = {
For DT based probing I need to modify the struct of_device_id, but I got
the idea :)
> {
> /* keep it for coldfire */
> .name = DRIVER_NAME,
> .driver_data = 0,
> }, {
> .name = "imx25-fec",
> .driver_data = FEC_QUIRK_USE_GASKET,
> }, {
> .name = "imx27-fec",
> .driver_data = 0,
> }, {
> .name = "imx28-fec",
> .driver_data = FEC_QUIRK_ENET_MAC | FEC_QUIRK_SWAP_FRAME,
> }, {
> .name = "imx6q-fec",
> .driver_data = FEC_QUIRK_ENET_MAC | FEC_QUIRK_HAS_GBIT |
> FEC_QUIRK_HAS_BUFDESC_EX | FEC_QUIRK_HAS_CSUM |
> FEC_QUIRK_HAS_VLAN | FEC_QUIRK_ERR006358,
> }, {
> .name = "mvf600-fec",
> .driver_data = FEC_QUIRK_ENET_MAC,
> }, {
> /* sentinel */
> }
> };
>
> So that we know which SoC needs to have the workaround applied or not.
I've already created a patch that rearranges the struct of_device_id in
the driver, works for me. I posted it (with another fix) for review on
linux-can[1].
Thanks,
Marc
[1] http://comments.gmane.org/gmane.linux.can/4050
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
Hi,
Marc Kleine-Budde writes:
> On 10/05/2013 07:46 PM, Fabio Estevam wrote:
> > Hi Marc,
> >
> > On Thu, Oct 3, 2013 at 3:51 PM, Marc Kleine-Budde <[email protected]> wrote:
> >
> >> For example:
> >>
> >> The imx28.dtsi gives this compatible string for its CAN core:
> >>
> >>> compatible = "fsl,imx28-flexcan", "fsl,p1010-flexcan";
> >>
> >> The flexcan driver defines:
> >>
> >>> static const struct of_device_id flexcan_of_match[] = {
> >>> { .compatible = "fsl,p1010-flexcan", .data = &fsl_p1010_devtype_data, },
> >>> { .compatible = "fsl,imx28-flexcan", .data = &fsl_imx28_devtype_data, },
> >>> { .compatible = "fsl,imx6q-flexcan", .data = &fsl_imx6q_devtype_data, },
> >>> { /* sentinel */ },
> >>> };
> >>
> >> The "p1010" was the first Freescale SoC with the flexcan core. But this SoC has
> >> a bug, so a workaround has to be enabled in the driver. The mx28 has this bug
> >> fixed, so we don't need this quite costly workaround.
> >
> > What about defining in imx28.dtsi:
> > compatible = "fsl,imx28-flexcan".
>
> It already works with changing only the driver.
>
IMO the change proposed by Fabio is much more sensible. If the imx28
implementation of the CAN controller is not compatible to p1010 (which
is obviously true) there is no point in having the "fsl,p1010-flexcan"
in the DT for imx28.
Lothar Waßmann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
http://www.karo-electronics.de | [email protected]
___________________________________________________________
On 10/07/2013 10:17 AM, Lothar Waßmann wrote:
> Hi,
>
> Marc Kleine-Budde writes:
>> On 10/05/2013 07:46 PM, Fabio Estevam wrote:
>>> Hi Marc,
>>>
>>> On Thu, Oct 3, 2013 at 3:51 PM, Marc Kleine-Budde <[email protected]> wrote:
>>>
>>>> For example:
>>>>
>>>> The imx28.dtsi gives this compatible string for its CAN core:
>>>>
>>>>> compatible = "fsl,imx28-flexcan", "fsl,p1010-flexcan";
>>>>
>>>> The flexcan driver defines:
>>>>
>>>>> static const struct of_device_id flexcan_of_match[] = {
>>>>> { .compatible = "fsl,p1010-flexcan", .data = &fsl_p1010_devtype_data, },
>>>>> { .compatible = "fsl,imx28-flexcan", .data = &fsl_imx28_devtype_data, },
>>>>> { .compatible = "fsl,imx6q-flexcan", .data = &fsl_imx6q_devtype_data, },
>>>>> { /* sentinel */ },
>>>>> };
>>>>
>>>> The "p1010" was the first Freescale SoC with the flexcan core. But this SoC has
>>>> a bug, so a workaround has to be enabled in the driver. The mx28 has this bug
>>>> fixed, so we don't need this quite costly workaround.
>>>
>>> What about defining in imx28.dtsi:
>>> compatible = "fsl,imx28-flexcan".
>>
>> It already works with changing only the driver.
>>
> IMO the change proposed by Fabio is much more sensible. If the imx28
> implementation of the CAN controller is not compatible to p1010 (which
> is obviously true) there is no point in having the "fsl,p1010-flexcan"
> in the DT for imx28.
But it is compatible with the "fsl,p1010-flexcan".
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |