Re: [PATCH v3 03/16] mfd: mfd-core: match device tree node against reg property

On Mon, 25 May 2020, Michael Walle wrote:

Am 2020-05-15 12:28, schrieb Lee Jones:

quoted

On Thu, 30 Apr 2020, Michael Walle wrote:

quoted

Hi Lee,

Am 2020-04-23 19:45, schrieb Michael Walle:

quoted

There might be multiple children with the device tree compatible, for
example if a MFD has multiple instances of the same function. In this
case only the first is matched and the other children get a wrong
of_node reference.
Add a new option to match also against the unit address of the child
node. Additonally, a new helper OF_MFD_CELL_REG is added.

Do you think this is feasible? I guess this is the biggest uncertainty
for me at the moment in this patch series.

I think it sounds fine in principle.  So long as it doesn't change the
existing behaviour when of_reg isn't set.

quoted

quoted

Signed-off-by: Michael Walle <redacted>
---
 drivers/mfd/mfd-core.c   | 29 ++++++++++++++++++++---------
 include/linux/mfd/core.h | 26 ++++++++++++++++++++------
 2 files changed, 40 insertions(+), 15 deletions(-)

[...]

quoted

quoted

quoted

diff --git a/include/linux/mfd/core.h b/include/linux/mfd/core.h
index d01d1299e49d..c2c0ad6b14f3 100644
--- a/include/linux/mfd/core.h
+++ b/include/linux/mfd/core.h

@@ -13,8 +13,11 @@
 #include <linux/platform_device.h>

 #define MFD_RES_SIZE(arr) (sizeof(arr) / sizeof(struct resource))
+#define MFD_OF_REG_VALID	BIT(31)

What about 64bit platforms?

The idea was to have this as a logical number. I.e. for now you may only
have one subdevice per unique compatible string. In fact, if you have a
look at the ab8500.c, there are multiple "stericsson,ab8500-pwm"
subdevices. But there is only one DT node for all three of it. I guess
this works as long as you don't use phandles to reference the pwm node
in the device tree. Or you don't want to use device tree properties
per subdevice (for example the "timeout-sec" of a watchdog device).

So to circumvent this, I thought of having the unit-address (and thus
the "reg" property) to differentiate between multiple subdevices. Now
there is one special case for me: this board management controller
might be upgradable and it might change internally. Thus I came up
with that logical numbering of subdevices. Rob doesn't seem to be a
fan of that, though. Therefore, having bit 31 as a valid indicator
leaves you with 2^31 logical devices, which should be enough ;)

Rob proposed to have the internal offset as the unit-address. But
in that case I can also use devm_of_platform_populate() and don't
need the OF_MFD_CELL_REG; I'd just parse the reg offset in each
individual subdevice driver. But like I said, I wanted to keep the
internal offsets out of the device tree.

Oh, I see what you're doing.

So you're adding an arbitrary ID to the device's reg property in DT?

How is this not a hack?

Why don't you use the full address for identification?

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel