Message ID | 532712D2.4020004@monstr.eu |
---|---|
State | New |
Headers | show |
On 03/17/2014 04:20 PM, Michal Simek wrote: > Hi, > > please pull these changes to your arm-soc tree. This branch is based > on zynq/dt branch. > Based on my discussion with Olof I have removed zynq-ocm driver > from this pull request and we will investigate different solution > > Changes in v3: Remove OCM driver from pull request > Changes in v2: Fix incorrect git repo url > > Thanks, > Michal > > > The following changes since commit 3795e91d2abb25164dda5687ca680a7ab940c447: > > arm: dt: zynq: Add fclk-enable property to clkc node (2014-02-03 17:00:24 +0100) > > are available in the git repository at: > > git://git.xilinx.com/linux-xlnx.git tags/zynq-cleanup-for-3.15-v2 > > for you to fetch changes up to bffc3b8d2197f5e0622a6f0afb30bffeb326f942: > > ARM: zynq: Add support for SOC_BUS (2014-03-17 15:13:25 +0100) > > ---------------------------------------------------------------- > arm: Xilinx Zynq cleanup patches for v3.15 > > - Redesign SLCR initialization to enable > driver developing which targets SLCR space > - Add SOC_BUS support > > ---------------------------------------------------------------- > Michal Simek (7): > ARM: zynq: Split slcr in two parts > ARM: zynq: Map I/O memory on clkc init > ARM: zynq: Add and use zynq_slcr_read/write() helper functions > ARM: zynq: Introduce zynq_slcr_unlock() > ARM: zynq: Move of_clk_init from clock driver > ARM: zynq: Add waituart implementation > ARM: zynq: Add support for SOC_BUS > > Steffen Trumtrar (3): > ARM: zynq: Move clock_init from slcr to common > ARM: zynq: Hang iomapped slcr address on device_node > ARM: zynq: Make zynq_slcr_base static > > Documentation/devicetree/bindings/clock/zynq-7000.txt | 4 ++- > arch/arm/boot/dts/zynq-7000.dtsi | 49 ++++++++++++++------------- > arch/arm/include/debug/zynq.S | 3 ++ > arch/arm/mach-zynq/Kconfig | 2 ++ > arch/arm/mach-zynq/common.c | 80 ++++++++++++++++++++++++++++++++++++++++++-- > arch/arm/mach-zynq/common.h | 3 +- > arch/arm/mach-zynq/slcr.c | 123 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------- > drivers/clk/zynq/clkc.c | 89 ++++++++++++++++++++++++++++++++++--------------- > include/linux/clk/zynq.h | 2 +- > 9 files changed, 285 insertions(+), 70 deletions(-) We are getting closer and closer to 3.14 and haven't got any reply on this one. Olof: Any update? Thanks, Michal
On Sun, Mar 23, 2014 at 11:50 PM, Michal Simek <monstr@monstr.eu> wrote: > On 03/17/2014 04:20 PM, Michal Simek wrote: >> Hi, >> >> please pull these changes to your arm-soc tree. This branch is based >> on zynq/dt branch. >> Based on my discussion with Olof I have removed zynq-ocm driver >> from this pull request and we will investigate different solution >> >> Changes in v3: Remove OCM driver from pull request >> Changes in v2: Fix incorrect git repo url >> >> Thanks, >> Michal >> >> >> The following changes since commit 3795e91d2abb25164dda5687ca680a7ab940c447: >> >> arm: dt: zynq: Add fclk-enable property to clkc node (2014-02-03 17:00:24 +0100) >> >> are available in the git repository at: >> >> git://git.xilinx.com/linux-xlnx.git tags/zynq-cleanup-for-3.15-v2 >> >> for you to fetch changes up to bffc3b8d2197f5e0622a6f0afb30bffeb326f942: >> >> ARM: zynq: Add support for SOC_BUS (2014-03-17 15:13:25 +0100) >> >> ---------------------------------------------------------------- >> arm: Xilinx Zynq cleanup patches for v3.15 >> >> - Redesign SLCR initialization to enable >> driver developing which targets SLCR space >> - Add SOC_BUS support >> >> ---------------------------------------------------------------- >> Michal Simek (7): >> ARM: zynq: Split slcr in two parts >> ARM: zynq: Map I/O memory on clkc init >> ARM: zynq: Add and use zynq_slcr_read/write() helper functions >> ARM: zynq: Introduce zynq_slcr_unlock() >> ARM: zynq: Move of_clk_init from clock driver >> ARM: zynq: Add waituart implementation >> ARM: zynq: Add support for SOC_BUS >> >> Steffen Trumtrar (3): >> ARM: zynq: Move clock_init from slcr to common >> ARM: zynq: Hang iomapped slcr address on device_node >> ARM: zynq: Make zynq_slcr_base static >> >> Documentation/devicetree/bindings/clock/zynq-7000.txt | 4 ++- >> arch/arm/boot/dts/zynq-7000.dtsi | 49 ++++++++++++++------------- >> arch/arm/include/debug/zynq.S | 3 ++ >> arch/arm/mach-zynq/Kconfig | 2 ++ >> arch/arm/mach-zynq/common.c | 80 ++++++++++++++++++++++++++++++++++++++++++-- >> arch/arm/mach-zynq/common.h | 3 +- >> arch/arm/mach-zynq/slcr.c | 123 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------- >> drivers/clk/zynq/clkc.c | 89 ++++++++++++++++++++++++++++++++++--------------- >> include/linux/clk/zynq.h | 2 +- >> 9 files changed, 285 insertions(+), 70 deletions(-) > > We are getting closer and closer to 3.14 and haven't got any reply on > this one. > Olof: Any update? Just me forgetting to deal with this merge, since it's been pending a while. -rc8 was just cut, it doesn't mean that we have time to take a lot of new material, but it does mean that we have time to deal with these ones that we forgot. I should be able to merge this tomorrow, either in the daytime or after I get to Napa for Collab Summit. -Olof
On Monday 17 March 2014, Michal Simek wrote: > please pull these changes to your arm-soc tree. This branch is based > on zynq/dt branch. > Based on my discussion with Olof I have removed zynq-ocm driver > from this pull request and we will investigate different solution > > Changes in v3: Remove OCM driver from pull request > Changes in v2: Fix incorrect git repo url I have pulled it into the next/cleanup2 branch now, sorry for the delay. Unfortunately I noticed one more thing I didn't like and did not pull the final patch of the branch, but only the other patches. The problem I noticed is the soc-bus support: I noticed that you are populating the entire device tree under the soc node, including any top-level devices, and that the "xlnx,zynq-devcfg-1.00.a" node is part of that. I think what you should try instead is to have only the amba bus and its children as part of the soc-bus, but other top-level devices (e.g. board clocks) outside of the soc node. Also, it would make sense to merge the devcfg stuff with the amba node, since it's really what makes up the soc. Does that make sense to you, or do you have reason to believe it won't work? Arnd
On 03/27/2014 02:28 AM, Arnd Bergmann wrote: > On Monday 17 March 2014, Michal Simek wrote: >> please pull these changes to your arm-soc tree. This branch is based >> on zynq/dt branch. >> Based on my discussion with Olof I have removed zynq-ocm driver >> from this pull request and we will investigate different solution >> >> Changes in v3: Remove OCM driver from pull request >> Changes in v2: Fix incorrect git repo url > > I have pulled it into the next/cleanup2 branch now, sorry for the > delay. Unfortunately I noticed one more thing I didn't like and > did not pull the final patch of the branch, but only the other patches. ok. thanks for pulling. > The problem I noticed is the soc-bus support: I noticed that you > are populating the entire device tree under the soc node, including > any top-level devices, and that the "xlnx,zynq-devcfg-1.00.a" > node is part of that. > > I think what you should try instead is to have only the > amba bus and its children as part of the soc-bus, but other > top-level devices (e.g. board clocks) outside of the soc > node. Also, it would make sense to merge the devcfg stuff > with the amba node, since it's really what makes up the > soc. Does that make sense to you, or do you have reason to > believe it won't work? No problem to postpone to the next release and discuss it more. Currently all devices listed in dts/dtsi are all hard IPs inside PS silicon that's why I think they should be the part of SoC. (zynq-devcfg is also hard IP present all the time in SoC I will use different compatible string in the next version xlnx,zynq-devcfg-1.0). For devices in PL is situation different because only axi-axi bridge is present in PS that's why I think that new bus type should be used. Also this bridge provides some resets which can be provided through this bus bridge driver. Current code do this: root@zynq:~# cat /sys/bus/soc/devices/soc0/revision 0x0 root@zynq:~# cat /sys/bus/soc/devices/soc0/soc_id 0x7 root@zynq:~# cat /sys/bus/soc/devices/soc0/family Xilinx Zynq root@zynq:~# ls /sys/bus/soc/devices/soc0/ amba.0 f8891000.pmu power soc_id uevent amba.1 family revision subsystem amba.0 is the bus present in zynq-7000.dtsi amba.1 is bus I have added myself just locally which is axi2axi bridge mentioned above. All devices which will be in PL will be connected to this amba.1 bus. Regarding clk driver in slcr node is not listed there. root@zynq:~# ls /sys/bus/soc/devices/soc0/amba.0/f8000000.slcr/ driver/ modalias power/ subsystem/ uevent Then we have also ACP (accelerator coherency port) which is the same case as axi2axi bridge. It means it is fixed hardware in SoC part and devices can be connect to it (It looks like arm-cci bus type). I know that others SoC are using soc {} node for adding soc IPs but that's not our case at least for now. If you think that SOC bus should just contain SoC specific things then we can call of_platform_populate without SoC bus parent to ensure that these devices are not listed there. Please correct me if something doesn't fit. Thanks, Michal
Hi Arnd, any response on this one? Thanks, Michal On 03/27/2014 12:41 PM, Michal Simek wrote: > On 03/27/2014 02:28 AM, Arnd Bergmann wrote: >> On Monday 17 March 2014, Michal Simek wrote: >>> please pull these changes to your arm-soc tree. This branch is based >>> on zynq/dt branch. >>> Based on my discussion with Olof I have removed zynq-ocm driver >>> from this pull request and we will investigate different solution >>> >>> Changes in v3: Remove OCM driver from pull request >>> Changes in v2: Fix incorrect git repo url >> >> I have pulled it into the next/cleanup2 branch now, sorry for the >> delay. Unfortunately I noticed one more thing I didn't like and >> did not pull the final patch of the branch, but only the other patches. > > ok. thanks for pulling. > >> The problem I noticed is the soc-bus support: I noticed that you >> are populating the entire device tree under the soc node, including >> any top-level devices, and that the "xlnx,zynq-devcfg-1.00.a" >> node is part of that. >> >> I think what you should try instead is to have only the >> amba bus and its children as part of the soc-bus, but other >> top-level devices (e.g. board clocks) outside of the soc >> node. Also, it would make sense to merge the devcfg stuff >> with the amba node, since it's really what makes up the >> soc. Does that make sense to you, or do you have reason to >> believe it won't work? > > No problem to postpone to the next release and discuss it more. > > Currently all devices listed in dts/dtsi are all hard IPs > inside PS silicon that's why I think they should be the part of SoC. > (zynq-devcfg is also hard IP present all the time in SoC > I will use different compatible string in the next version > xlnx,zynq-devcfg-1.0). > > For devices in PL is situation different because only axi-axi bridge > is present in PS that's why I think that new bus type should > be used. Also this bridge provides some resets which can be > provided through this bus bridge driver. > > Current code do this: > > root@zynq:~# cat /sys/bus/soc/devices/soc0/revision > 0x0 > root@zynq:~# cat /sys/bus/soc/devices/soc0/soc_id > 0x7 > root@zynq:~# cat /sys/bus/soc/devices/soc0/family > Xilinx Zynq > root@zynq:~# ls /sys/bus/soc/devices/soc0/ > amba.0 f8891000.pmu power soc_id uevent > amba.1 family revision subsystem > > amba.0 is the bus present in zynq-7000.dtsi > amba.1 is bus I have added myself just locally which is > axi2axi bridge mentioned above. > > All devices which will be in PL will be connected to this amba.1 > bus. > > Regarding clk driver in slcr node is not listed there. > > root@zynq:~# ls /sys/bus/soc/devices/soc0/amba.0/f8000000.slcr/ > driver/ modalias power/ subsystem/ uevent > > > Then we have also ACP (accelerator coherency port) which is the > same case as axi2axi bridge. It means it is fixed hardware in SoC > part and devices can be connect to it > (It looks like arm-cci bus type). > > I know that others SoC are using soc {} node for adding > soc IPs but that's not our case at least for now. > > If you think that SOC bus should just contain SoC specific > things then we can call of_platform_populate without SoC bus > parent to ensure that these devices are not listed there. > > Please correct me if something doesn't fit. > > Thanks, > Michal > >
On Thursday 27 March 2014, Michal Simek wrote: > > The problem I noticed is the soc-bus support: I noticed that you > > are populating the entire device tree under the soc node, including > > any top-level devices, and that the "xlnx,zynq-devcfg-1.00.a" > > node is part of that. > > > > I think what you should try instead is to have only the > > amba bus and its children as part of the soc-bus, but other > > top-level devices (e.g. board clocks) outside of the soc > > node. Also, it would make sense to merge the devcfg stuff > > with the amba node, since it's really what makes up the > > soc. Does that make sense to you, or do you have reason to > > believe it won't work? > > No problem to postpone to the next release and discuss it more. > > Currently all devices listed in dts/dtsi are all hard IPs > inside PS silicon that's why I think they should be the part of SoC. > (zynq-devcfg is also hard IP present all the time in SoC > I will use different compatible string in the next version > xlnx,zynq-devcfg-1.0). > > For devices in PL is situation different because only axi-axi bridge > is present in PS that's why I think that new bus type should > be used. Also this bridge provides some resets which can be > provided through this bus bridge driver. > > Current code do this: > > root@zynq:~# cat /sys/bus/soc/devices/soc0/revision > 0x0 > root@zynq:~# cat /sys/bus/soc/devices/soc0/soc_id > 0x7 > root@zynq:~# cat /sys/bus/soc/devices/soc0/family > Xilinx Zynq > root@zynq:~# ls /sys/bus/soc/devices/soc0/ > amba.0 f8891000.pmu power soc_id uevent > amba.1 family revision subsystem > > amba.0 is the bus present in zynq-7000.dtsi > amba.1 is bus I have added myself just locally which is > axi2axi bridge mentioned above. > > All devices which will be in PL will be connected to this amba.1 > bus. > > Regarding clk driver in slcr node is not listed there. > > root@zynq:~# ls /sys/bus/soc/devices/soc0/amba.0/f8000000.slcr/ > driver/ modalias power/ subsystem/ uevent > > > Then we have also ACP (accelerator coherency port) which is the > same case as axi2axi bridge. It means it is fixed hardware in SoC > part and devices can be connect to it > (It looks like arm-cci bus type). > > I know that others SoC are using soc {} node for adding > soc IPs but that's not our case at least for now. > > If you think that SOC bus should just contain SoC specific > things then we can call of_platform_populate without SoC bus > parent to ensure that these devices are not listed there. > > Please correct me if something doesn't fit. I don't have a strong opinion on things that are defined in FPGA logic, they could be below soc0, in a separate soc1 node or in some other top-level bus. What I was mostly thinking of is board-level devices that are not part of the soc address hierarchy, such as external clocks or leds that just use GPIO lines. Arnd
On 04/25/2014 11:00 AM, Arnd Bergmann wrote: > On Thursday 27 March 2014, Michal Simek wrote: >>> The problem I noticed is the soc-bus support: I noticed that you >>> are populating the entire device tree under the soc node, including >>> any top-level devices, and that the "xlnx,zynq-devcfg-1.00.a" >>> node is part of that. >>> >>> I think what you should try instead is to have only the >>> amba bus and its children as part of the soc-bus, but other >>> top-level devices (e.g. board clocks) outside of the soc >>> node. Also, it would make sense to merge the devcfg stuff >>> with the amba node, since it's really what makes up the >>> soc. Does that make sense to you, or do you have reason to >>> believe it won't work? >> >> No problem to postpone to the next release and discuss it more. >> >> Currently all devices listed in dts/dtsi are all hard IPs >> inside PS silicon that's why I think they should be the part of SoC. >> (zynq-devcfg is also hard IP present all the time in SoC >> I will use different compatible string in the next version >> xlnx,zynq-devcfg-1.0). >> >> For devices in PL is situation different because only axi-axi bridge >> is present in PS that's why I think that new bus type should >> be used. Also this bridge provides some resets which can be >> provided through this bus bridge driver. >> >> Current code do this: >> >> root@zynq:~# cat /sys/bus/soc/devices/soc0/revision >> 0x0 >> root@zynq:~# cat /sys/bus/soc/devices/soc0/soc_id >> 0x7 >> root@zynq:~# cat /sys/bus/soc/devices/soc0/family >> Xilinx Zynq >> root@zynq:~# ls /sys/bus/soc/devices/soc0/ >> amba.0 f8891000.pmu power soc_id uevent >> amba.1 family revision subsystem >> >> amba.0 is the bus present in zynq-7000.dtsi >> amba.1 is bus I have added myself just locally which is >> axi2axi bridge mentioned above. >> >> All devices which will be in PL will be connected to this amba.1 >> bus. >> >> Regarding clk driver in slcr node is not listed there. >> >> root@zynq:~# ls /sys/bus/soc/devices/soc0/amba.0/f8000000.slcr/ >> driver/ modalias power/ subsystem/ uevent >> >> >> Then we have also ACP (accelerator coherency port) which is the >> same case as axi2axi bridge. It means it is fixed hardware in SoC >> part and devices can be connect to it >> (It looks like arm-cci bus type). >> >> I know that others SoC are using soc {} node for adding >> soc IPs but that's not our case at least for now. >> >> If you think that SOC bus should just contain SoC specific >> things then we can call of_platform_populate without SoC bus >> parent to ensure that these devices are not listed there. >> >> Please correct me if something doesn't fit. > > I don't have a strong opinion on things that are defined in FPGA > logic, they could be below soc0, in a separate soc1 node or > in some other top-level bus. ok. > What I was mostly thinking of is board-level devices that are > not part of the soc address hierarchy, such as external clocks > or leds that just use GPIO lines. Then let's discuss this based on our zynq dtsi. GIC, PL310, (OCM controller - we will discuss this later), UARTS, CADENCE GEM, ARASAN MMC, A9 global timer, TTC timer, SCUTIMER - all of these are just the part of the soc address hierarchy and shouldn't be any problem with them to be under soc0 - all of them are in silicon. DEVCFG - it is the part of first group - it is hard IP which can program programmable logic. Address is FIXED just wanted to list it separately. I2C - I have just sent pull request for adding it there. i2c childs/devices are added to the i2c node based on binding that's why this should be fine. PMU - Currently I have it in root but maybe better location should be used. SLCR - which is our system controller where everything is together. There is just ps-clk-frequency which is user setup and come externally. The part of this is clock driver which is probably that problematic one but on the other hand it is targetting exact register addresses which are used by this driver to describe internal clk description. I have looked at ux500 platform which has also SOC_BUS and based your thinking for example cpufreq-cooling, regulator-gpio shouldn't be there and probably more than this. Regarding clock description there is even not register range specified that's why it should be easily moved out of soc bus. Linus: It is not a complain just want it to get it right. If you have any example how this should be done, please send it to me. I would like to have this in good shape. Thanks, Michal
On Friday 25 April 2014 11:49:07 Michal Simek wrote: > On 04/25/2014 11:00 AM, Arnd Bergmann wrote: > > On Thursday 27 March 2014, Michal Simek wrote: > > > What I was mostly thinking of is board-level devices that are > > not part of the soc address hierarchy, such as external clocks > > or leds that just use GPIO lines. > > Then let's discuss this based on our zynq dtsi. > GIC, PL310, (OCM controller - we will discuss this later), UARTS, CADENCE GEM, > ARASAN MMC, A9 global timer, TTC timer, SCUTIMER > - all of these are just the part of the soc address hierarchy and shouldn't > be any problem with them to be under soc0 - all of them are in silicon. Yes, sounds good. > DEVCFG - it is the part of first group - it is hard IP which can program > programmable logic. Address is FIXED just wanted to list it separately. > > I2C - I have just sent pull request for adding it there. > i2c childs/devices are added to the i2c node based on binding that's why > this should be fine. > > PMU - Currently I have it in root but maybe better location should be used. > > SLCR - which is our system controller where everything is together. I think these should also all be considered part of the SoC. > There is just ps-clk-frequency which is user setup and come externally. > The part of this is clock driver which is probably that problematic one > but on the other hand it is targetting exact register addresses which > are used by this driver to describe internal clk description. If you just have a 'ps-clk-frequency' property, that should still keep the node inside of the SoC. If this had been modeled as a fixed-rate clock node as the clock parent of the xlnx,ps7-clkc node, that one should have been outside. I don't think it's worth changing it now, just something to keep in mind. > I have looked at ux500 platform which has also SOC_BUS and > based your thinking for example cpufreq-cooling, regulator-gpio shouldn't > be there and probably more than this. Regarding clock description there > is even not register range specified that's why it should be easily moved > out of soc bus. Good point. > If you have any example how this should be done, please send it to me. > I would like to have this in good shape. >From all I can tell, all the nodes you have today actually make sense being part of the soc device hierarchy. The only problem is that there is no way to add nodes outside of it if that becomes necessary. Generally speaking, the hierarchy should reflect the actual hardware layout, which usually has multiple levels of nested buses, starting with an AXI bus that contains the high-speed devices along with bridges to lower-speed buses. You probably have access to documentation that describes the real layout, so try to match that to the degree that you can, and move the pmu into whatever bus you need. Then change the code to match the top-level bus node with the soc0 device. Arnd
>> There is just ps-clk-frequency which is user setup and come externally. >> The part of this is clock driver which is probably that problematic one >> but on the other hand it is targetting exact register addresses which >> are used by this driver to describe internal clk description. > > If you just have a 'ps-clk-frequency' property, that should still keep > the node inside of the SoC. If this had been modeled as a fixed-rate > clock node as the clock parent of the xlnx,ps7-clkc node, that one > should have been outside. > > I don't think it's worth changing it now, just something to keep in > mind. ok. >> If you have any example how this should be done, please send it to me. >> I would like to have this in good shape. > > From all I can tell, all the nodes you have today actually make sense being > part of the soc device hierarchy. The only problem is that there is no > way to add nodes outside of it if that becomes necessary. > > Generally speaking, the hierarchy should reflect the actual hardware layout, > which usually has multiple levels of nested buses, starting with an AXI > bus that contains the high-speed devices along with bridges to lower-speed > buses. You probably have access to documentation that describes the > real layout, so try to match that to the degree that you can, and move the > pmu into whatever bus you need. > > Then change the code to match the top-level bus node with the soc0 device. For static devices which we have now is axi bus for soft IPs just nested bus and should be added to soc0. That means not a problem with current layout at all and this SOC_BUS patch can be just added. And I will take care when we have something what it is out of SoC that it is also added to the proper location out of this soc node. Can I add you ACK to this SOC_BUS patch? Thanks, Michal