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
