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.
--
Robert Hancock
Senior Software Developer
SED Systems, a division of Calian Ltd.
Email: hanc...@sedsystems.ca
