History

mfd: core: Make a best effort attempt to match devices with the correct of_nodes

Currently, when a child platform device (sometimes referred to as a sub-device) is registered via the Multi-Functional Device (MFD) API, the framework attempts to match the newly registered platform device with its associated Device Tree (OF) node. Until now, the device has been allocated the first node found with an identical OF compatible string. Unfortunately, if there are, say for example '3' devices which are to be handled by the same driver and therefore have the same compatible string, each of them will be allocated a pointer to the *first* node. An example Device Tree entry might look like this: mfd_of_test { compatible = "mfd,of-test-parent"; #address-cells = <0x02>; #size-cells = <0x02>; child@aaaaaaaaaaaaaaaa { compatible = "mfd,of-test-child"; reg = <0xaaaaaaaa 0xaaaaaaaa 0 0x11>, <0xbbbbbbbb 0xbbbbbbbb 0 0x22>; }; child@cccccccc { compatible = "mfd,of-test-child"; reg = <0x00000000 0xcccccccc 0 0x33>; }; child@dddddddd00000000 { compatible = "mfd,of-test-child"; reg = <0xdddddddd 0x00000000 0 0x44>; }; }; When used with example sub-device registration like this: static const struct mfd_cell mfd_of_test_cell[] = { OF_MFD_CELL("mfd-of-test-child", NULL, NULL, 0, 0, "mfd,of-test-child"), OF_MFD_CELL("mfd-of-test-child", NULL, NULL, 0, 1, "mfd,of-test-child"), OF_MFD_CELL("mfd-of-test-child", NULL, NULL, 0, 2, "mfd,of-test-child") }; ... the current implementation will result in all devices being allocated the first OF node found containing a matching compatible string: [0.712511] mfd-of-test-child mfd-of-test-child.0: Probing platform device: 0 [0.712710] mfd-of-test-child mfd-of-test-child.0: Using OF node: child@aaaaaaaaaaaaaaaa [0.713033] mfd-of-test-child mfd-of-test-child.1: Probing platform device: 1 [0.713381] mfd-of-test-child mfd-of-test-child.1: Using OF node: child@aaaaaaaaaaaaaaaa [0.713691] mfd-of-test-child mfd-of-test-child.2: Probing platform device: 2 [0.713889] mfd-of-test-child mfd-of-test-child.2: Using OF node: child@aaaaaaaaaaaaaaaa After this patch each device will be allocated a unique OF node: [0.712511] mfd-of-test-child mfd-of-test-child.0: Probing platform device: 0 [0.712710] mfd-of-test-child mfd-of-test-child.0: Using OF node: child@aaaaaaaaaaaaaaaa [0.713033] mfd-of-test-child mfd-of-test-child.1: Probing platform device: 1 [0.713381] mfd-of-test-child mfd-of-test-child.1: Using OF node: child@cccccccc [0.713691] mfd-of-test-child mfd-of-test-child.2: Probing platform device: 2 [0.713889] mfd-of-test-child mfd-of-test-child.2: Using OF node: child@dddddddd00000000 Which is fine if all OF nodes are identical. However if we wish to apply an attribute to particular device, we really need to ensure the correct OF node will be associated with the device containing the correct address. We accomplish this by matching the device's address expressed in DT with one provided during sub-device registration. Like this: static const struct mfd_cell mfd_of_test_cell[] = { OF_MFD_CELL_REG("mfd-of-test-child", NULL, NULL, 0, 1, "mfd,of-test-child", 0xdddddddd00000000), OF_MFD_CELL_REG("mfd-of-test-child", NULL, NULL, 0, 2, "mfd,of-test-child", 0xaaaaaaaaaaaaaaaa), OF_MFD_CELL_REG("mfd-of-test-child", NULL, NULL, 0, 3, "mfd,of-test-child", 0x00000000cccccccc) }; This will ensure a specific device (designated here using the platform_ids; 1, 2 and 3) is matched with a particular OF node: [0.712511] mfd-of-test-child mfd-of-test-child.0: Probing platform device: 0 [0.712710] mfd-of-test-child mfd-of-test-child.0: Using OF node: child@dddddddd00000000 [0.713033] mfd-of-test-child mfd-of-test-child.1: Probing platform device: 1 [0.713381] mfd-of-test-child mfd-of-test-child.1: Using OF node: child@aaaaaaaaaaaaaaaa [0.713691] mfd-of-test-child mfd-of-test-child.2: Probing platform device: 2 [0.713889] mfd-of-test-child mfd-of-test-child.2: Using OF node: child@cccccccc This implementation is still not infallible, hence the mention of "best effort" in the commit subject. Since we have not *insisted* on the existence of 'reg' properties (in some scenarios they just do not make sense) and no device currently uses the new 'of_reg' attribute, we have to make an on-the-fly judgement call whether to associate the OF node anyway. Which we do in cases where parent drivers haven't specified a particular OF node to match to. So there is a *slight* possibility of the following result (note: the implementation here is convoluted, but it shows you one means by which this process can still break): /* * First entry will match to the first OF node with matching compatible * Second will fail, since the first took its OF node and is no longer available * Third will succeed */ static const struct mfd_cell mfd_of_test_cell[] = { OF_MFD_CELL("mfd-of-test-child", NULL, NULL, 0, 1, "mfd,of-test-child"), OF_MFD_CELL_REG("mfd-of-test-child", NULL, NULL, 0, 2, "mfd,of-test-child", 0xaaaaaaaaaaaaaaaa), OF_MFD_CELL_REG("mfd-of-test-child", NULL, NULL, 0, 3, "mfd,of-test-child", 0x00000000cccccccc) }; The result: [0.753869] mfd-of-test-parent mfd_of_test: Registering 3 devices [0.756597] mfd-of-test-child: Failed to locate of_node [id: 2] [0.759999] mfd-of-test-child mfd-of-test-child.1: Probing platform device: 1 [0.760314] mfd-of-test-child mfd-of-test-child.1: Using OF node: child@aaaaaaaaaaaaaaaa [0.760908] mfd-of-test-child mfd-of-test-child.2: Probing platform device: 2 [0.761183] mfd-of-test-child mfd-of-test-child.2: No OF node associated with this device [0.761621] mfd-of-test-child mfd-of-test-child.3: Probing platform device: 3 [0.761899] mfd-of-test-child mfd-of-test-child.3: Using OF node: child@cccccccc We could code around this with some pre-parsing semantics, but the added complexity required to cover each and every corner-case is not justified. Merely patching the current failing (via this patch) is already working with some pretty small corner-cases. Other issues should be patched in the parent drivers which can be achieved simply by implementing OF_MFD_CELL_REG().

Lee Jones committed 6 years ago

Commit 466a62d1 parent 46b57802 files · +93 −12Browse files

Unified · Split

2 files
  1. drivers/mfd/mfd-core.c +83 −12
  2. include/linux/mfd/core.h +10 −0
drivers/mfd/mfd-core.c+83−12
@@ -10,6 +10,7 @@ · 9 unmodified lines
1010 #include <linux/kernel.h>
1111 #include <linux/platform_device.h>
1212 #include <linux/acpi.h>
13+#include <linux/list.h>
1314 #include <linux/property.h>
1415 #include <linux/mfd/core.h>
1516 #include <linux/pm_runtime.h>
@@ -17,8 +18,17 @@ · 1 unmodified line
1718 #include <linux/module.h>
1819 #include <linux/irqdomain.h>
1920 #include <linux/of.h>
21+#include <linux/of_address.h>
2022 #include <linux/regulator/consumer.h>
2123
24+static LIST_HEAD(mfd_of_node_list);
25+
26+struct mfd_of_node_entry {
27+ struct list_head list;
28+ struct device *dev;
29+ struct device_node *np;
30+};
31+
2232 static struct device_type mfd_dev_type = {
2333 .name = "mfd_device",
2434 };
@@ -107,6 +117,55 @@ static inline void mfd_acpi_add_device(const struct mfd_cell *cell, · 82 unmodified lines
107117 }
108118 #endif
109119
120+static int mfd_match_of_node_to_dev(struct platform_device *pdev,
121+ struct device_node *np,
122+ const struct mfd_cell *cell)
123+{
124+#if IS_ENABLED(CONFIG_OF)
125+ struct mfd_of_node_entry *of_entry;
126+ const __be32 *reg;
127+ u64 of_node_addr;
128+
129+ /* Skip devices 'disabled' by Device Tree */
130+ if (!of_device_is_available(np))
131+ return -ENODEV;
132+
133+ /* Skip if OF node has previously been allocated to a device */
134+ list_for_each_entry(of_entry, &mfd_of_node_list, list)
135+ if (of_entry->np == np)
136+ return -EAGAIN;
137+
138+ if (!cell->use_of_reg)
139+ /* No of_reg defined - allocate first free compatible match */
140+ goto allocate_of_node;
141+
142+ /* We only care about each node's first defined address */
143+ reg = of_get_address(np, 0, NULL, NULL);
144+ if (!reg)
145+ /* OF node does not contatin a 'reg' property to match to */
146+ return -EAGAIN;
147+
148+ of_node_addr = of_read_number(reg, of_n_addr_cells(np));
149+
150+ if (cell->of_reg != of_node_addr)
151+ /* No match */
152+ return -EAGAIN;
153+
154+allocate_of_node:
155+ of_entry = kzalloc(sizeof(*of_entry), GFP_KERNEL);
156+ if (!of_entry)
157+ return -ENOMEM;
158+
159+ of_entry->dev = &pdev->dev;
160+ of_entry->np = np;
161+ list_add_tail(&of_entry->list, &mfd_of_node_list);
162+
163+ pdev->dev.of_node = np;
164+ pdev->dev.fwnode = &np->fwnode;
165+#endif
166+ return 0;
167+}
168+
110169 static int mfd_add_device(struct device *parent, int id,
111170 const struct mfd_cell *cell,
112171 struct resource *mem_base,
@@ -115,6 +174,7 @@ static int mfd_add_device(struct device *parent, int id, · 2 unmodified lines
115174 struct resource *res;
116175 struct platform_device *pdev;
117176 struct device_node *np = NULL;
177+ struct mfd_of_node_entry *of_entry, *tmp;
118178 int ret = -ENOMEM;
119179 int platform_id;
120180 int r;
@@ -149,19 +209,22 @@ static int mfd_add_device(struct device *parent, int id, · 28 unmodified lines
149209 if (ret < 0)
150210 goto fail_res;
151211
152− if (parent->of_node && cell->of_compatible) {
212+ if (IS_ENABLED(CONFIG_OF) && parent->of_node && cell->of_compatible) {
153213 for_each_child_of_node(parent->of_node, np) {
154214 if (of_device_is_compatible(np, cell->of_compatible)) {
155− if (!of_device_is_available(np)) {
156− /* Ignore disabled devices error free */
157− ret = 0;
215+ ret = mfd_match_of_node_to_dev(pdev, np, cell);
216+ if (ret == -EAGAIN)
217+ continue;
218+ if (ret)
158219 goto fail_alias;
159− }
160− pdev->dev.of_node = np;
161− pdev->dev.fwnode = &np->fwnode;
220+
162221 break;
163222 }
164223 }
224+
225+ if (!pdev->dev.of_node)
226+ pr_warn("%s: Failed to locate of_node [id: %d]\n",
227+ cell->name, platform_id);
165228 }
166229
167230 mfd_acpi_add_device(cell, pdev);
@@ -170,13 +233,13 @@ static int mfd_add_device(struct device *parent, int id, · 2 unmodified lines
170233 ret = platform_device_add_data(pdev,
171234 cell->platform_data, cell->pdata_size);
172235 if (ret)
173− goto fail_alias;
236+ goto fail_of_entry;
174237 }
175238
176239 if (cell->properties) {
177240 ret = platform_device_add_properties(pdev, cell->properties);
178241 if (ret)
179− goto fail_alias;
242+ goto fail_of_entry;
180243 }
181244
182245 for (r = 0; r < cell->num_resources; r++) {
@@ -213,18 +276,18 @@ static int mfd_add_device(struct device *parent, int id, · 30 unmodified lines
213276 if (has_acpi_companion(&pdev->dev)) {
214277 ret = acpi_check_resource_conflict(&res[r]);
215278 if (ret)
216− goto fail_alias;
279+ goto fail_of_entry;
217280 }
218281 }
219282 }
220283
221284 ret = platform_device_add_resources(pdev, res, cell->num_resources);
222285 if (ret)
223− goto fail_alias;
286+ goto fail_of_entry;
224287
225288 ret = platform_device_add(pdev);
226289 if (ret)
227− goto fail_alias;
290+ goto fail_of_entry;
228291
229292 if (cell->pm_runtime_no_callbacks)
230293 pm_runtime_no_callbacks(&pdev->dev);
@@ -233,6 +296,12 @@ static int mfd_add_device(struct device *parent, int id, · 2 unmodified lines
233296
234297 return 0;
235298
299+fail_of_entry:
300+ list_for_each_entry_safe(of_entry, tmp, &mfd_of_node_list, list)
301+ if (of_entry->dev == &pdev->dev) {
302+ list_del(&of_entry->list);
303+ kfree(of_entry);
304+ }
236305 fail_alias:
237306 regulator_bulk_unregister_supply_alias(&pdev->dev,
238307 cell->parent_supplies,
@@ -297,6 +366,8 @@ static int mfd_remove_devices_fn(struct device *dev, void *data) · 58 unmodified lines
297366 regulator_bulk_unregister_supply_alias(dev, cell->parent_supplies,
298367 cell->num_parent_supplies);
299368
369+ kfree(cell);
370+
300371 platform_device_unregister(pdev);
301372 return 0;
302373 }
include/linux/mfd/core.h+10−0
@@ -78,6 +78,16 @@ struct mfd_cell { · 77 unmodified lines
7878 */
7979 const char *of_compatible;
8080
81+ /*
82+ * Address as defined in Device Tree. Used to compement 'of_compatible'
83+ * (above) when matching OF nodes with devices that have identical
84+ * compatible strings
85+ */
86+ const u64 of_reg;
87+
88+ /* Set to 'true' to use 'of_reg' (above) - allows for of_reg=0 */
89+ bool use_of_reg;
90+
8191 /* Matches ACPI */
8292 const struct mfd_cell_acpi_match *acpi_match;
8393