On Wed, 24 Jun 2020, Michael Walle wrote: > Hi, > > Am 2020-06-24 08:41, schrieb Lee Jones: > > On Tue, 23 Jun 2020, Frank Rowand wrote: > > > > > On 2020-06-11 14:10, Lee Jones wrote: > > > > 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. > > > > > > As you mentioned elsewhere in this thread, this series "fixes" the > > > problem related to the "stericsson,ab8500-pwm" compatible. > > > > > > I know, I said I would drop discussion of that compatible, but bear > > > with me for a second. :-) > > > > > > The "problem" is that the devices for multiple mfd child nodes with > > > the same compatible value of "stericsson,ab8500-pwm" will all have > > > a pointer to the first child node. At the moment the same child > > > of_node being used by more than one device does not cause any > > > incorrect behavior. > > > > > > Just in case the driver for "stericsson,ab8500-pwm" is modified > > > in a way that the child of_node needs to be distinct for each > > > device, and that changes gets back ported, it would be useful > > > to have Fixes: tags for this patch series. > > > > > > So, at your discretion (and I'll let you worry about the correct > > > Fixes: tag format), this series fixes: > > > > > > bad76991d7847b7877ae797cc79745d82ffd9120 mfd: Register ab8500 > > > devices using the newly DT:ed MFD API > > > > This patch isn't actually broken. > > > > The issue is the DTB, which [0] addresses. > > > > [0] > > https://lkml.kernel.org/lkml/20200622083432.1491715-1-lee.jones@xxxxxxxxxx/ > > Now, I'm confused; because this patch doesn't use the reg property > but a different node name. The fix mentioned above is orthogonal to this set. The *only* reason for the differing node names there is to circumvent the following DTC warnings: arch/arm/boot/dts/ste-ab8500.dtsi:210.16-214.7: ERROR (duplicate_node_names): /soc/prcmu@80157000/ab8500/ab8500-pwm: Duplicate node name arch/arm/boot/dts/ste-ab8500.dtsi:216.16-220.7: ERROR (duplicate_node_names): /soc/prcmu@80157000/ab8500/ab8500-pwm: Duplicate node name arch/arm/boot/dts/ste-ab8500.dtsi:216.16-220.7: ERROR (duplicate_node_names): /soc/prcmu@80157000/ab8500/ab8500-pwm: Duplicate node name > I'd actually prefer this for any MFD > driver which has multiple nodes of the same compatible string. See > my reasoning here [1]. But until now, no one has responded. Thus, > I'd rather see a OF_MFD_CELL_NAME() which matches the node name > instead of the OF_MFD_CELL_REG() macro. > > This would also circumvent the fact that the unit-address has one > number space. Eg. it is not possible to have: > > mfd { > compatible = "mfd,compatible"; > > gpio@0 { > reg = <0>; > }; > gpio@1 { > reg = <1>; > }; > pwm@0 { > reg = <0>; > }; > }; > > Although Rob mentioned to maybe relax that, but I sill fail to see > the advantage to have an arbitrary reg property instead of a unique > node name. I don't have a strong opinion either way. We can *also* add node name matching if Rob deems it fit. -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
