On Wed, 05 Jun 2019, Robert Hancock wrote: > 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 <hanc...@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.
What is contained in each of their resources? -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog