[PATCH 0/2] MFD core updates for device tree binding support

[PATCH 0/2] MFD core updates for device tree binding support

[Robert Hancock]

Fixes for the device tree binding support in MFD core.

Robert Hancock (2):
  mfd: core: Support multiple OF child devices of the same type
  mfd: core: Set fwnode for created devices

 drivers/mfd/mfd-core.c   | 6 +++++-
 include/linux/mfd/core.h | 3 +++
 2 files changed, 8 insertions(+), 1 deletion(-)

-- 
1.8.3.1

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

[Robert Hancock]

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;
 			}
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;
+
 	/* Matches ACPI */
 	const struct mfd_cell_acpi_match	*acpi_match;
 
-- 
1.8.3.1

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

[Lee Jones]

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.

> 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?

>  	/* Matches ACPI */
>  	const struct mfd_cell_acpi_match	*acpi_match;
>  

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

[Robert Hancock]

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

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

[Lee Jones]

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 <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.

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

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

[Robert Hancock]

On 2019-06-05 12:45 p.m., Lee Jones wrote:
>>>> 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?

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

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@sedsystems.ca

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

[Lee Jones]

On Wed, 05 Jun 2019, Robert Hancock wrote:

> On 2019-06-05 12:45 p.m., Lee Jones wrote:
> >>>> 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?
> 
> 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.

> 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.
> 

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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@sedsystems.ca

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

[Lee Jones]

On Thu, 06 Jun 2019, Robert Hancock wrote:

> 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.

You need to call of_platform_populate().

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 2/2] mfd: core: Set fwnode for created devices

[Robert Hancock]

The logic for setting the of_node on devices created by mfd did not set
the fwnode pointer to match, which caused fwnode-based APIs to
malfunction on these devices since the fwnode pointer was null. Fix
this.

Signed-off-by: Robert Hancock <hancock@sedsystems.ca>
---
 drivers/mfd/mfd-core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
index 74bc895..228163c 100644
--- a/drivers/mfd/mfd-core.c
+++ b/drivers/mfd/mfd-core.c
@@ -182,6 +182,7 @@ static int mfd_add_device(struct device *parent, int id,
 			     !strcmp(cell->of_full_name,
 				     of_node_full_name(np)))) {
 				pdev->dev.of_node = np;
+				pdev->dev.fwnode = &np->fwnode;
 				break;
 			}
 		}
-- 
1.8.3.1

Re: [PATCH 2/2] mfd: core: Set fwnode for created devices

[Lee Jones]

On Tue, 04 Jun 2019, Robert Hancock wrote:

> The logic for setting the of_node on devices created by mfd did not set
> the fwnode pointer to match, which caused fwnode-based APIs to
> malfunction on these devices since the fwnode pointer was null. Fix
> this.
> 
> Signed-off-by: Robert Hancock <hancock@sedsystems.ca>
> ---
>  drivers/mfd/mfd-core.c | 1 +
>  1 file changed, 1 insertion(+)

Applied, thanks.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog