Messages in this thread | | | Subject | Re: [PATCH 1/2] mfd: core: Support multiple OF child devices of the same type | From | Robert Hancock <> | Date | Wed, 5 Jun 2019 10:23:36 -0600 |
| |
On 2019-06-05 12:31 a.m., Lee Jones wrote: > On Tue, 04 Jun 2019, Robert Hancock wrote: > >> Previously the MFD core supported assigning OF nodes to created MFD >> devices, but relied solely on matching the of_compatible string. This >> would result in devices being potentially assigned the wrong node if >> there are multiple devices with the same compatible string within a >> multifunction device. >> >> Add support for matching the full name of the node in the MFD cell >> definition, so that we can match against a specific instance of a >> device. If this is not specified, we match just based on the >> compatible string, as before. >> >> Signed-off-by: Robert Hancock <hancock@sedsystems.ca> >> --- >> drivers/mfd/mfd-core.c | 5 ++++- >> include/linux/mfd/core.h | 3 +++ >> 2 files changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c >> index 1ade4c8..74bc895 100644 >> --- a/drivers/mfd/mfd-core.c >> +++ b/drivers/mfd/mfd-core.c >> @@ -177,7 +177,10 @@ static int mfd_add_device(struct device *parent, int id, >> >> if (parent->of_node && cell->of_compatible) { >> for_each_child_of_node(parent->of_node, np) { >> - if (of_device_is_compatible(np, cell->of_compatible)) { >> + if (of_device_is_compatible(np, cell->of_compatible) && >> + (!cell->of_full_name || >> + !strcmp(cell->of_full_name, >> + of_node_full_name(np)))) { >> pdev->dev.of_node = np; >> break; > > That is some ugly, squashed up code. > > If we end up accepting this, I suggest flattening this out a bit. > > ... but we'll cross that bridge when we come to it.
Yes, that if statement could be broken up to make it more readable. Will fix in a next version assuming the concept is acceptable.
> >> diff --git a/include/linux/mfd/core.h b/include/linux/mfd/core.h >> index 99c0395..470f6cb 100644 >> --- a/include/linux/mfd/core.h >> +++ b/include/linux/mfd/core.h >> @@ -55,6 +55,9 @@ struct mfd_cell { >> */ >> const char *of_compatible; >> >> + /* Optionally match against a specific device of a given type */ >> + const char *of_full_name; >> + > > Can you give me an example for when this might be useful?
This is an example of some device tree entries for our MFD device:
axi_iic_0: i2c@c0000 { compatible = "xlnx,xps-iic-2.00.a"; clocks = <&axi_clk>; clock-frequency = <100000>; interrupts = <7>; #size-cells = <0>; #address-cells = <1>; };
axi_iic_1: i2c@d0000 { compatible = "xlnx,xps-iic-2.00.a"; clocks = <&axi_clk>; clock-frequency = <100000>; interrupts = <8>; #size-cells = <0>; #address-cells = <1>; };
and the corresponding MFD cells:
{ .name = "axi_iic_0", .of_compatible = "xlnx,xps-iic-2.00.a", .of_full_name = "i2c@c0000", .num_resources = ARRAY_SIZE(dbe_i2c1_resources), .resources = dbe_i2c1_resources }, { .name = "axi_iic_1", .of_compatible = "xlnx,xps-iic-2.00.a", .of_full_name = "i2c@d0000", .num_resources = ARRAY_SIZE(dbe_i2c2_resources), .resources = dbe_i2c2_resources },
Without having the .of_full_name support, both MFD cells ended up wrongly matching against the i2c@c0000 device tree node since we just picked the first one where of_compatible matched.
-- Robert Hancock Senior Software Developer SED Systems, a division of Calian Ltd. Email: hancock@sedsystems.ca
| |