On 5/6/2014 6:32 PM, Rob Herring wrote: > On Tue, May 6, 2014 at 7:48 PM, Frank Rowand <frowand.list@xxxxxxxxx> wrote: >> An issue with the path of SPMI nodes under /sys/bus/... was reported in >> https://lkml.org/lkml/2014/4/23/312. The symptom is that two different < snip > >> > > I think the primary question to ask is there any added benefit to > having the additional hierarchy of devices. I don't think there is > much support to have more hierarchy from what I have seen of past > discussions. The hierarchy avoids the name space conflict. It also maps the physical reality better than pretending all devices are on the platform bus. It follows the model that non-device tree systems use. For example, why should a usb device show up under /sys/bus/platform/ instead of under /sys/bus/usb/ ? (I'm not positive this actually happens, but let me pretend it would.) > Another approach could be to support having multiple platform bus > instances. Then drivers can easily create a new instance for each set > of sub-devices. > > This can be solved in a much less invasive way just in the DT naming > algorithm. This is slightly different from what I had suggested of > just dropping the unit address. It keeps the unit address, but adds > the unique index on untranslate-able addresses. The diff is bigger due > to refactoring to reduce the indentation levels. It is untested and > whitespace corrupted: > > diff --git a/drivers/of/platform.c b/drivers/of/platform.c > index 404d1da..c77dd7a 100644 > --- a/drivers/of/platform.c > +++ b/drivers/of/platform.c > @@ -105,23 +105,33 @@ void of_device_make_bus_id(struct device *dev) > * For MMIO, get the physical address > */ > reg = of_get_property(node, "reg", NULL); > - if (reg) { > - if (of_can_translate_address(node)) { > - addr = of_translate_address(node, reg); > - } else { > - addrp = of_get_address(node, 0, NULL, NULL); > - if (addrp) > - addr = of_read_number(addrp, 1); > - else > - addr = OF_BAD_ADDR; > - } > - if (addr != OF_BAD_ADDR) { > - dev_set_name(dev, "%llx.%s", > - (unsigned long long)addr, node->name); > - return; > - } > + if (!reg) > + goto no_bus_id; > + > + if (of_can_translate_address(node)) { > + addr = of_translate_address(node, reg); > + if (addr == OF_BAD_ADDR) > + goto no_bus_id; > + > + dev_set_name(dev, "%llx.%s", > + (unsigned long long)addr, node->name); > + return; > } > > + addrp = of_get_address(node, 0, NULL, NULL); > + if (!addrp) > + goto no_bus_id; > + > + addr = of_read_number(addrp, 1); > + if (addr == OF_BAD_ADDR) > + goto no_bus_id; > + > + magic = atomic_add_return(1, &bus_no_reg_magic); > + dev_set_name(dev, "%llx.%s.%d", (unsigned long long)addr, > + node->name, magic - 1); > + return; > + > +no_bus_id: > /* > * No BusID, use the node name and add a globally incremented > * counter (and pray...) > I think the refactored code is easier to read. (End of bike shed...) The result of that patch is an even uglier set of device names. I would love to get rid of the bus_no_reg_magic instead of making more extensive use of it. The new names for the system that started this discussion are: $ ls /sys/devices/soc.2/fc4cf000.qcom,spmi/spmi-0/0-00/ 100.qcom,revid.19 gpios.21 3100.qcom,pm8x41-adc-usr.20 power 6000.qcom,rtc.18 subsystem driver uevent $ ls /sys/bus/platform/devices/soc.2/fc4cf000.qcom,spmi/spmi-0/0-00/ 100.qcom,revid.19 gpios.21 3100.qcom,pm8x41-adc-usr.20 power 6000.qcom,rtc.18 subsystem driver uevent $ ls /sys/bus/platform/devices/soc.2/fc4cf000.qcom,spmi/spmi-0/0-01/ b040.pm8xxx-pwm.22 driver uevent d000.pm8xxx-pwm-led.23 power d800.pm8xxx-wled.24 subsystem $ ls /sys/bus/platform/devices/soc.2/fc4cf000.qcom,spmi/spmi-0/0-04/ 100.qcom,revid.25 power uevent driver subsystem $ ls /sys/bus/platform/devices/ 100.qcom,revid.19 fc4ab000.restart 100.qcom,revid.25 fc4cf000.qcom,spmi 3100.qcom,pm8x41-adc-usr.20 fd484000.hwlock 6000.qcom,rtc.18 fd510000.pinctrl alarmtimer fd8c0000.clock-controller b040.pm8xxx-pwm.22 gpio_keys.5 cpu-pmu.1 gpios.21 cpus.0 iio-thermal.4 d000.pm8xxx-pwm-led.23 pm8841-s1.6 d800.pm8xxx-wled.24 pm8841-s2.7 f9000000.interrupt-controller pm8941-l3.11 f9011000.smem pm8941-l6.12 f9012000.regulator reg-dummy f9020000.timer regulator-l11.14 f9088000.clock-controller regulator-l19.15 f9098000.clock-controller regulator-l20.16 f90a8000.clock-controller regulator-l22.17 f90b8000.clock-controller regulator-l9.13 f9824900.sdhc regulator-s1.8 f991e000.serial regulator-s2.9 f9924000.i2c2 regulator-s3.10 f9928000.i2c6 regulatory.0 f9bff000.rng snd-soc-dummy fc400000.clock-controller soc.2 fc4281d0.qcom,mpm timer.3 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
