From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754873AbcEYPTR (ORCPT ); Wed, 25 May 2016 11:19:17 -0400 Received: from mout.kundenserver.de ([212.227.17.10]:61254 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751853AbcEYPTO (ORCPT ); Wed, 25 May 2016 11:19:14 -0400 From: Arnd Bergmann To: Rob Herring Cc: Sanchayan Maity , Shawn Guo , Stefan Agner , "linux-arm-kernel@lists.infradead.org" , "devicetree@vger.kernel.org" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH v3 4/4] soc: Add SoC driver for Freescale Vybrid platform Date: Wed, 25 May 2016 17:18:22 +0200 Message-ID: <5922111.afqro67xTI@wuerfel> User-Agent: KMail/5.1.3 (Linux/4.4.0-22-generic; KDE/5.18.0; x86_64; ; ) In-Reply-To: References: <20160524041447.GA2884@Sanchayan-Arch.localdomain> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:HBd19Vr5M+lMIbKcyz5k0EzbZIKAjan4iWMNgfXk3dEswaTc+wu sjY75KALz8Jd/EvIbkoSIu2xNU7NaJhQbuVGzf3rgLF3ZGJWEmbW63cT5kRcGCfFDiIza9n jeOwobjPLNtxZrGaYZp0KP4OSJqfM3BloEJ67NtHorx0nc47Um7DlfZcCReO7e1rtbsYGIZ o30eQZnIXc0adLu3aQtmw== X-UI-Out-Filterresults: notjunk:1;V01:K0:r+lM4dIuXM8=:7dnNCuNxM6313PRelYSDSB Gl6msF0c/9dOhYtR/BtTc5VQ/lwiSCdOxIfYYm1dPNlY8coU+zmHNsxPQGSoqDExVVzML0Ao9 OnDYZ/PxW+m77PrlG3NoJgegXHhJd+oPh/ihVkThwTt5UP2y8I62CKvcjnCvTS83962uVqd3u ulQnZ8VqbCYCOgTG6JU3nmWO90ugn+/bFxqMzer2wOFKf6oFQ6ojh5YtOcyHvvAbkNugbWO7i R8pGgnd+EbE588TIA3dsTLHVjFd45h5U7lwoSWWDPpDX7X4RKMH9jJzP+FH5EvvDsAir/5RAx RfCE9hb6/PQUFdo5YiD1IfAgyfWsBGXD4cCiPxsQKhD2vhRp0QFjIPt6Ke4iBy2WgAxFM3HtJ KIM2nGH3cjUAaOJY+EXZCv3CMRO0RVZSw65QSoE6HbdveUTv2YSc/JY5pa9Ayjb7zWj1qBxmh IcU3R2x7TZwZ8UUmMpTOZKmF7EJLeoXKZfuWJoBcJfPPs1cgyTuYjpZLKDAEg0zVSbCBxQ56H OSfnJVITN+iCgyOOoCDglCDUKtXpEhLfSqTiachk0Bfs1wSnZFkCIJISlzFpOKzm1n/raT085 utqW5AokaEj7gYI7gi/X5NfTONDcxRZHa3IBaB3snJiplmhOXuT7CnqWDbH9QqAtNnQwKEbNV 5abi7Ql8Tq5lcIADDKS8k9DexWxqjBP9b4Zl1wXwxciRYd7yxFf6g8GGnpiclZgS8XZyXyy6W F2Rwyg9okv5O9Clh Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tuesday, May 24, 2016 12:09:41 PM CEST Rob Herring wrote: > On Mon, May 23, 2016 at 11:14 PM, wrote: > > Hello Rob, > > > > On 16-05-23 16:18:13, Rob Herring wrote: > >> On Fri, May 20, 2016 at 03:32:05PM +0530, Sanchayan Maity wrote: > >> > This adds a SoC driver to be used by Freescale Vybrid SoC's. > >> > Driver utilises syscon and nvmem consumer API's to get the > >> > various register values needed and expose the SoC specific > >> > properties via sysfs. > >> > > >> > A sample output from Colibri Vybrid VF61 is below: > >> > > >> > root@colibri-vf:~# cd /sys/bus/soc/devices/soc0 > >> > root@colibri-vf:/sys/bus/soc/devices/soc0# ls > >> > family machine power revision soc_id subsystem uevent > >> > root@colibri-vf:/sys/bus/soc/devices/soc0# cat family > >> > Freescale Vybrid VF610 > >> > root@colibri-vf:/sys/bus/soc/devices/soc0# cat machine > >> > Freescale Vybrid > >> > root@colibri-vf:/sys/bus/soc/devices/soc0# cat revision > >> > 00000013 > >> > root@colibri-vf:/sys/bus/soc/devices/soc0# cat soc_id > >> > df6472a6130f29d4 > >> > > >> > Signed-off-by: Sanchayan Maity > >> > --- > >> > .../bindings/arm/freescale/fsl,vf610-soc.txt | 20 +++ > >> > drivers/soc/Kconfig | 1 + > >> > drivers/soc/fsl/Kconfig | 10 ++ > >> > drivers/soc/fsl/Makefile | 1 + > >> > drivers/soc/fsl/soc-vf610.c | 198 +++++++++++++++++++++ > >> > 5 files changed, 230 insertions(+) > >> > create mode 100644 Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt > >> > create mode 100644 drivers/soc/fsl/Kconfig > >> > create mode 100644 drivers/soc/fsl/soc-vf610.c > >> > > >> > diff --git a/Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt b/Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt > >> > new file mode 100644 > >> > index 0000000..338905d > >> > --- /dev/null > >> > +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt > >> > @@ -0,0 +1,20 @@ > >> > +Vybrid System-on-Chip > >> > +--------------------- > >> > + > >> > +Required properties: > >> > + > >> > +- compatible: "fsl,vf610-soc" > >> > +- rom-revision: phandle to the on-chip ROM node > >> > +- mscm: phandle to the MSCM CPU configuration node > >> > +- nvmem-cells: phandles to two OCOTP child nodes ocotp_cfg0 and ocotp_cfg1 > >> > +- nvmem-cell-names: should contain string names "cfg0" and "cfg1" > >> > >> I still have similar concerns as the discussion on the last version. > >> This version only proves that you aren't describing h/w, but rather just > >> a collection of data that some driver wants. > >> > >> A driver can just as easily look-up all the nodes directly that these > >> phandles point to. > > > > Agreed, that we can look up all the nodes directly that these phandles > > refer to but I would still need a DT entry to bind to. While I could > > bind to existing nodes like mscm cpucfg but that doesn't seem right. > > > > The very first approach that we had taken was to integrate this functionality > > in mach-vf610.c code under mach-imx > > http://www.spinics.net/lists/devicetree/msg80654.html > > Yes, everyone wants to move all platform devices in the kernel to a > corresponding DT node. The result is often making up nodes to do this. > It's the same thing with cpufreq. > > > and then it was recommended to migrate this to drivers/soc where we did use > > phandles or direct look up via compatible strings > > The location in the tree is an orthogonal issue. You could move it and > use of_machine_is_compatible without any DT change. > > The primary issue I have here is how do we bind soc_bus to DT in a > consistent way. Sorry, but vybrid specific patches alone are never > going to solve that issue. On a lot of chips, there is actually one device that holds the soc identification registers, and that can easily be used for this purpose. Originally, the soc_device was meant to be the parent device for all other on-chip devices and be bound to the DT node that holds that bus (unlike things like external clocks that are not part of the SoC). However, that model has never really caught on, and we have stopped doing this for new chips. Instead, the soc_device now somewhat stands for itself. > > http://www.spinics.net/lists/arm-kernel/msg420847.html > > > > and > > > > http://lkml.iu.edu/hypermail/linux/kernel/1506.0/03787.html > > > > There hasn't been a consensus since v1. > > I actually prefer your previous version binding soc_bus to the root > bus node to this version. I think that is closer to the right > direction. After all, pretty much everything is an SOC and every SOC > has an SOC bus. Pretty much every SOC and board have revisions and may > need to expose that revision info as well. We have to do this > consistently which means having a default implementation for > simple-bus that is not opt-in. > > Alternatively, we should just deprecate soc_bus and come up a > different solution. Either way, I think we have a half implemented > solution currently. We have had several drivers in the past that needed to figure out whether they were running on a particular version of a SoC, and that could not get this information from the DT. My suggested solution for this is to provide an API from soc_bus that compares a glob string to the available soc_device instance(s) in order to figure out whether we are running on a particular kind of system. Arnd From mboxrd@z Thu Jan 1 00:00:00 1970 From: Arnd Bergmann Subject: Re: [PATCH v3 4/4] soc: Add SoC driver for Freescale Vybrid platform Date: Wed, 25 May 2016 17:18:22 +0200 Message-ID: <5922111.afqro67xTI@wuerfel> References: <20160524041447.GA2884@Sanchayan-Arch.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7Bit Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Rob Herring Cc: Sanchayan Maity , Shawn Guo , Stefan Agner , "linux-arm-kernel@lists.infradead.org" , "devicetree@vger.kernel.org" , "linux-kernel@vger.kernel.org" List-Id: devicetree@vger.kernel.org On Tuesday, May 24, 2016 12:09:41 PM CEST Rob Herring wrote: > On Mon, May 23, 2016 at 11:14 PM, wrote: > > Hello Rob, > > > > On 16-05-23 16:18:13, Rob Herring wrote: > >> On Fri, May 20, 2016 at 03:32:05PM +0530, Sanchayan Maity wrote: > >> > This adds a SoC driver to be used by Freescale Vybrid SoC's. > >> > Driver utilises syscon and nvmem consumer API's to get the > >> > various register values needed and expose the SoC specific > >> > properties via sysfs. > >> > > >> > A sample output from Colibri Vybrid VF61 is below: > >> > > >> > root@colibri-vf:~# cd /sys/bus/soc/devices/soc0 > >> > root@colibri-vf:/sys/bus/soc/devices/soc0# ls > >> > family machine power revision soc_id subsystem uevent > >> > root@colibri-vf:/sys/bus/soc/devices/soc0# cat family > >> > Freescale Vybrid VF610 > >> > root@colibri-vf:/sys/bus/soc/devices/soc0# cat machine > >> > Freescale Vybrid > >> > root@colibri-vf:/sys/bus/soc/devices/soc0# cat revision > >> > 00000013 > >> > root@colibri-vf:/sys/bus/soc/devices/soc0# cat soc_id > >> > df6472a6130f29d4 > >> > > >> > Signed-off-by: Sanchayan Maity > >> > --- > >> > .../bindings/arm/freescale/fsl,vf610-soc.txt | 20 +++ > >> > drivers/soc/Kconfig | 1 + > >> > drivers/soc/fsl/Kconfig | 10 ++ > >> > drivers/soc/fsl/Makefile | 1 + > >> > drivers/soc/fsl/soc-vf610.c | 198 +++++++++++++++++++++ > >> > 5 files changed, 230 insertions(+) > >> > create mode 100644 Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt > >> > create mode 100644 drivers/soc/fsl/Kconfig > >> > create mode 100644 drivers/soc/fsl/soc-vf610.c > >> > > >> > diff --git a/Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt b/Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt > >> > new file mode 100644 > >> > index 0000000..338905d > >> > --- /dev/null > >> > +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt > >> > @@ -0,0 +1,20 @@ > >> > +Vybrid System-on-Chip > >> > +--------------------- > >> > + > >> > +Required properties: > >> > + > >> > +- compatible: "fsl,vf610-soc" > >> > +- rom-revision: phandle to the on-chip ROM node > >> > +- mscm: phandle to the MSCM CPU configuration node > >> > +- nvmem-cells: phandles to two OCOTP child nodes ocotp_cfg0 and ocotp_cfg1 > >> > +- nvmem-cell-names: should contain string names "cfg0" and "cfg1" > >> > >> I still have similar concerns as the discussion on the last version. > >> This version only proves that you aren't describing h/w, but rather just > >> a collection of data that some driver wants. > >> > >> A driver can just as easily look-up all the nodes directly that these > >> phandles point to. > > > > Agreed, that we can look up all the nodes directly that these phandles > > refer to but I would still need a DT entry to bind to. While I could > > bind to existing nodes like mscm cpucfg but that doesn't seem right. > > > > The very first approach that we had taken was to integrate this functionality > > in mach-vf610.c code under mach-imx > > http://www.spinics.net/lists/devicetree/msg80654.html > > Yes, everyone wants to move all platform devices in the kernel to a > corresponding DT node. The result is often making up nodes to do this. > It's the same thing with cpufreq. > > > and then it was recommended to migrate this to drivers/soc where we did use > > phandles or direct look up via compatible strings > > The location in the tree is an orthogonal issue. You could move it and > use of_machine_is_compatible without any DT change. > > The primary issue I have here is how do we bind soc_bus to DT in a > consistent way. Sorry, but vybrid specific patches alone are never > going to solve that issue. On a lot of chips, there is actually one device that holds the soc identification registers, and that can easily be used for this purpose. Originally, the soc_device was meant to be the parent device for all other on-chip devices and be bound to the DT node that holds that bus (unlike things like external clocks that are not part of the SoC). However, that model has never really caught on, and we have stopped doing this for new chips. Instead, the soc_device now somewhat stands for itself. > > http://www.spinics.net/lists/arm-kernel/msg420847.html > > > > and > > > > http://lkml.iu.edu/hypermail/linux/kernel/1506.0/03787.html > > > > There hasn't been a consensus since v1. > > I actually prefer your previous version binding soc_bus to the root > bus node to this version. I think that is closer to the right > direction. After all, pretty much everything is an SOC and every SOC > has an SOC bus. Pretty much every SOC and board have revisions and may > need to expose that revision info as well. We have to do this > consistently which means having a default implementation for > simple-bus that is not opt-in. > > Alternatively, we should just deprecate soc_bus and come up a > different solution. Either way, I think we have a half implemented > solution currently. We have had several drivers in the past that needed to figure out whether they were running on a particular version of a SoC, and that could not get this information from the DT. My suggested solution for this is to provide an API from soc_bus that compares a glob string to the available soc_device instance(s) in order to figure out whether we are running on a particular kind of system. Arnd From mboxrd@z Thu Jan 1 00:00:00 1970 From: arnd@arndb.de (Arnd Bergmann) Date: Wed, 25 May 2016 17:18:22 +0200 Subject: [PATCH v3 4/4] soc: Add SoC driver for Freescale Vybrid platform In-Reply-To: References: <20160524041447.GA2884@Sanchayan-Arch.localdomain> Message-ID: <5922111.afqro67xTI@wuerfel> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Tuesday, May 24, 2016 12:09:41 PM CEST Rob Herring wrote: > On Mon, May 23, 2016 at 11:14 PM, wrote: > > Hello Rob, > > > > On 16-05-23 16:18:13, Rob Herring wrote: > >> On Fri, May 20, 2016 at 03:32:05PM +0530, Sanchayan Maity wrote: > >> > This adds a SoC driver to be used by Freescale Vybrid SoC's. > >> > Driver utilises syscon and nvmem consumer API's to get the > >> > various register values needed and expose the SoC specific > >> > properties via sysfs. > >> > > >> > A sample output from Colibri Vybrid VF61 is below: > >> > > >> > root at colibri-vf:~# cd /sys/bus/soc/devices/soc0 > >> > root at colibri-vf:/sys/bus/soc/devices/soc0# ls > >> > family machine power revision soc_id subsystem uevent > >> > root at colibri-vf:/sys/bus/soc/devices/soc0# cat family > >> > Freescale Vybrid VF610 > >> > root at colibri-vf:/sys/bus/soc/devices/soc0# cat machine > >> > Freescale Vybrid > >> > root at colibri-vf:/sys/bus/soc/devices/soc0# cat revision > >> > 00000013 > >> > root at colibri-vf:/sys/bus/soc/devices/soc0# cat soc_id > >> > df6472a6130f29d4 > >> > > >> > Signed-off-by: Sanchayan Maity > >> > --- > >> > .../bindings/arm/freescale/fsl,vf610-soc.txt | 20 +++ > >> > drivers/soc/Kconfig | 1 + > >> > drivers/soc/fsl/Kconfig | 10 ++ > >> > drivers/soc/fsl/Makefile | 1 + > >> > drivers/soc/fsl/soc-vf610.c | 198 +++++++++++++++++++++ > >> > 5 files changed, 230 insertions(+) > >> > create mode 100644 Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt > >> > create mode 100644 drivers/soc/fsl/Kconfig > >> > create mode 100644 drivers/soc/fsl/soc-vf610.c > >> > > >> > diff --git a/Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt b/Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt > >> > new file mode 100644 > >> > index 0000000..338905d > >> > --- /dev/null > >> > +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,vf610-soc.txt > >> > @@ -0,0 +1,20 @@ > >> > +Vybrid System-on-Chip > >> > +--------------------- > >> > + > >> > +Required properties: > >> > + > >> > +- compatible: "fsl,vf610-soc" > >> > +- rom-revision: phandle to the on-chip ROM node > >> > +- mscm: phandle to the MSCM CPU configuration node > >> > +- nvmem-cells: phandles to two OCOTP child nodes ocotp_cfg0 and ocotp_cfg1 > >> > +- nvmem-cell-names: should contain string names "cfg0" and "cfg1" > >> > >> I still have similar concerns as the discussion on the last version. > >> This version only proves that you aren't describing h/w, but rather just > >> a collection of data that some driver wants. > >> > >> A driver can just as easily look-up all the nodes directly that these > >> phandles point to. > > > > Agreed, that we can look up all the nodes directly that these phandles > > refer to but I would still need a DT entry to bind to. While I could > > bind to existing nodes like mscm cpucfg but that doesn't seem right. > > > > The very first approach that we had taken was to integrate this functionality > > in mach-vf610.c code under mach-imx > > http://www.spinics.net/lists/devicetree/msg80654.html > > Yes, everyone wants to move all platform devices in the kernel to a > corresponding DT node. The result is often making up nodes to do this. > It's the same thing with cpufreq. > > > and then it was recommended to migrate this to drivers/soc where we did use > > phandles or direct look up via compatible strings > > The location in the tree is an orthogonal issue. You could move it and > use of_machine_is_compatible without any DT change. > > The primary issue I have here is how do we bind soc_bus to DT in a > consistent way. Sorry, but vybrid specific patches alone are never > going to solve that issue. On a lot of chips, there is actually one device that holds the soc identification registers, and that can easily be used for this purpose. Originally, the soc_device was meant to be the parent device for all other on-chip devices and be bound to the DT node that holds that bus (unlike things like external clocks that are not part of the SoC). However, that model has never really caught on, and we have stopped doing this for new chips. Instead, the soc_device now somewhat stands for itself. > > http://www.spinics.net/lists/arm-kernel/msg420847.html > > > > and > > > > http://lkml.iu.edu/hypermail/linux/kernel/1506.0/03787.html > > > > There hasn't been a consensus since v1. > > I actually prefer your previous version binding soc_bus to the root > bus node to this version. I think that is closer to the right > direction. After all, pretty much everything is an SOC and every SOC > has an SOC bus. Pretty much every SOC and board have revisions and may > need to expose that revision info as well. We have to do this > consistently which means having a default implementation for > simple-bus that is not opt-in. > > Alternatively, we should just deprecate soc_bus and come up a > different solution. Either way, I think we have a half implemented > solution currently. We have had several drivers in the past that needed to figure out whether they were running on a particular version of a SoC, and that could not get this information from the DT. My suggested solution for this is to provide an API from soc_bus that compares a glob string to the available soc_device instance(s) in order to figure out whether we are running on a particular kind of system. Arnd