Re: [PATCH 1/2] mfd: core: Support multiple OF child devices of the same type

[Robert Hancock]

On 2019-06-05 11:27 p.m., Lee Jones wrote:
>>>> 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?
>>
>> These are the resource entries for those two devices:
>>
>> static const struct resource dbe_i2c1_resources[] = {
>> {
>> 	.start		= 0xc0000,
>> 	.end		= 0xcffff,
>> 	.name		= "xi2c1_regs",
>> 	.flags		= IORESOURCE_MEM,
>> 	.desc		= IORES_DESC_NONE
>> },
>> };
>>
>> static const struct resource dbe_i2c2_resources[] = {
>> {
>> 	.start		= 0xd0000,
>> 	.end		= 0xdffff,
>> 	.name		= "xi2c2_regs",
>> 	.flags		= IORESOURCE_MEM,
>> 	.desc		= IORES_DESC_NONE
>> },
>> };
> 
> This is your problem.  You are providing the memory resources through
> *both* DT and MFD.  I don't believe I've seen your MFD driver, but it
> looks like it's probably not required at all.  Just allow DT to probe
> each of your child devices.  You can obtain the IO memory from there
> directly using the usual platform_get_resource() calls.

As far as I can tell, the DT child devices underneath a PCIe device
don't get probed and drivers loaded automatically - possibly for valid
reasons. The MFD driver appears to be required in order to actually get
drivers attached to those DT nodes.

Right now those devices are ending up with no memory resources unless
they are injected through the MFD cells. It would be handy if the memory
resources were mapped automatically from the PCIe BARs to the
sub-devices, to avoid duplicating information in the DT and the driver,
but even if that was solved it wouldn't avoid the need for this patch,
as the devices would still end up attached to the wrong DT node and pick
up the wrong properties.

The other reason we need the MFD driver is we are implementing an IRQ
domain to map the interrupts from the PCIe device to the child nodes,
and using some of those callbacks to poke other registers on the PCIe to
assist with converting the level-triggered AXI interrupts to
edge-triggered MSIs.

This is what the outer DT leading up to what I showed earlier looks like.

&pcie {
    pinctrl-names = "default";
    pinctrl-0 = <&pinctrl_pcie>;
    reset-gpio = <&gpio7 12 GPIO_ACTIVE_LOW>;
    status = "okay";

    pci_rootport: pcie@0,0 {
        reg = <0x83000000 0 0 0 0>;
        #address-cells = <3>;
        #size-cells = <2>;
        ranges;

        fpga_pcie: pcie@1,0 {
            reg = <0x2010000 0 0 0 0>;
            #address-cells = <1>;
            #size-cells = <1>;
            interrupt-controller;
            #interrupt-cells = <1>;

            ...

            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>;
            };
        };
    };
};



> 
>> Ideally the IO memory resource entries would be picked up and mapped
>> through the device tree as well, as they are with the interrupts, but I
>> haven't yet found the device tree magic that would allow that to happen
>> yet, if it's possible. The setup we have has a number of peripherals on
>> an AXI bus which are behind a PCIe to AXI bridge, and we're using mfd to
>> instantiate each of those AXI devices under the PCIe device.
>>
> 

-- 
Robert Hancock
Senior Software Developer
SED Systems, a division of Calian Ltd.
Email: hancock@xxxxxxxxxxxxx