On Wed, Apr 18, 2018 at 3:28 PM, Jae Hyun Yoo <jae.hyun....@linux.intel.com> wrote: > On 4/18/2018 7:32 AM, Rob Herring wrote: >> >> On Tue, Apr 17, 2018 at 3:40 PM, Jae Hyun Yoo >> <jae.hyun....@linux.intel.com> wrote: >>> >>> On 4/16/2018 4:51 PM, Jae Hyun Yoo wrote: >>>> >>>> >>>> On 4/16/2018 4:22 PM, Jae Hyun Yoo wrote: >>>>> >>>>> >>>>> On 4/16/2018 11:14 AM, Rob Herring wrote: >>>>>> >>>>>> >>>>>> On Tue, Apr 10, 2018 at 11:32:09AM -0700, Jae Hyun Yoo wrote: >>>>>>> >>>>>>> >>>>>>> This commit adds dt-bindings documents for PECI cputemp and dimmtemp >>>>>>> client >>>>>>> drivers. >>>>>> >>>>>> >>>>>> >>> >>> [...] >>> >>>>>>> +Example: >>>>>>> + peci-bus@0 { >>>>>>> + #address-cells = <1>; >>>>>>> + #size-cells = <0>; >>>>>>> + < more properties > >>>>>>> + >>>>>>> + peci-dimmtemp@cpu0 { >>>>>> >>>>>> >>>>>> >>>>>> unit-address is wrong. >>>>>> >>>>> >>>>> Will fix it using the reg value. >>>>> >>>>>> It is a different bus from cputemp? Otherwise, you have conflicting >>>>>> addresses. If that's the case, probably should make it clear by >>>>>> showing >>>>>> different host adapters for each example. >>>>>> >>>>> >>>>> It could be the same bus with cputemp. Also, client address sharing is >>>>> possible by PECI core if the functionality is different. I mean, >>>>> cputemp and >>>>> dimmtemp targeting the same client is possible case like this. >>>>> peci-cputemp@30 >>>>> peci-dimmtemp@30 >>>>> >>>> >>>> Oh, I got your point. Probably, I should change these separate settings >>>> into one like >>>> >>>> peci-client@30 { >>>> compatible = "intel,peci-client"; >>>> reg = <0x30>; >>>> }; >>>> >>>> Then cputemp and dimmtemp drivers could refer the same compatible >>>> string. >>>> Will rewrite it. >>>> >>> >>> I've checked it again and realized that it should use function based node >>> name like: >>> >>> peci-cputemp@30 >>> peci-dimmtemp@30 >>> >>> If it use the same string like 'peci-client@30', the drivers cannot be >>> selectively enabled. The client address sharing way is well handled in >>> PECI >>> core and this way would be better for the future implementations of other >>> PECI functional drivers such as crash dump driver and so on. So I'm going >>> change the unit-address only. >> >> >> 2 nodes at the same address is wrong (and soon dtc will warn you on >> this). You have 2 potential options. The first is you need additional >> address information in the DT if these are in fact 2 independent >> devices. This could be something like a function number to use >> something from PCI addressing. From what I found on PECI, it doesn't >> seem to have anything like that. The 2nd option is you have a single >> DT node which registers multiple hwmon devices. DT nodes and drivers >> don't have to be 1-1. Don't design your DT nodes from how you want to >> partition drivers in some OS. >> >> Rob >> > > Please correct me if I'm wrong but I'm still thinking that it is > possible. Also, I did compile it but dtc doesn't make a warning. Let me > show an another use case which is similar to this case:
I did say *soon*. It's in dtc repo, but not the kernel copy yet. > In arch/arm/boot/dts/aspeed-g5.dtsi > [...] > lpc_host: lpc-host@80 { > compatible = "aspeed,ast2500-lpc-host", "simple-mfd", "syscon"; > reg = <0x80 0x1e0>; > reg-io-width = <4>; > > #address-cells = <1>; > #size-cells = <1>; > ranges = <0x0 0x80 0x1e0>; > > lpc_ctrl: lpc-ctrl@0 { > compatible = "aspeed,ast2500-lpc-ctrl"; > reg = <0x0 0x80>; > clocks = <&syscon ASPEED_CLK_GATE_LCLK>; > status = "disabled"; > }; > > lpc_snoop: lpc-snoop@0 { > compatible = "aspeed,ast2500-lpc-snoop"; > reg = <0x0 0x80>; > interrupts = <8>; > status = "disabled"; > }; > } > [...] > > This is device tree setting for LPC interface and its child nodes. > LPC interface can be used as a multi-functional interface such as > snoop 80, KCS, SIO and so on. In this use case, lpc-ctrl@0 and > lpc-snoop@0 are sharing their address range from their individual > driver modules and they can be registered quite well through both > static dt or dynamic dtoverlay. PECI is also a multi-functional > interface which is similar to the above case, I think. This case too is poor design and should be fixed as well. Simply put, you can have 2 devices on a bus at the same address without some sort of mux or arbitration device in the middle. If you have a device/block with multiple functions provided to the OS, then it is the OS's problem to arbitrate access. It is not a DT problem because OS's can vary in how they handle that both from OS to OS and over time. Rob -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html