diff mbox

[v3,4/4] soc: Add SoC driver for Freescale Vybrid platform

Message ID ffef165d05f71a1b19437171d8fdb4bc78f6b1e3.1463737502.git.maitysanchayan@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Sanchayan May 20, 2016, 10:02 a.m. UTC
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 <maitysanchayan@gmail.com>
---
 .../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

Comments

Rob Herring (Arm) May 23, 2016, 9:18 p.m. UTC | #1
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 <maitysanchayan@gmail.com>
> ---
>  .../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.

And as long as we have inconsistent use of soc_device, I don't want to 
see any compatible strings related to it.

Rob
Sanchayan May 24, 2016, 4:14 a.m. UTC | #2
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 <maitysanchayan@gmail.com>
> > ---
> >  .../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

and then it was recommended to migrate this to drivers/soc where we did use
phandles or direct look up via compatible strings

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.

> 
> And as long as we have inconsistent use of soc_device, I don't want to 
> see any compatible strings related to it.
> 

Regards,
Sanchayan.
Rob Herring (Arm) May 24, 2016, 5:09 p.m. UTC | #3
On Mon, May 23, 2016 at 11:14 PM,  <maitysanchayan@gmail.com> 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 <maitysanchayan@gmail.com>
>> > ---
>> >  .../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.

> 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.

Rob
Arnd Bergmann May 25, 2016, 3:18 p.m. UTC | #4
On Tuesday, May 24, 2016 12:09:41 PM CEST Rob Herring wrote:
> On Mon, May 23, 2016 at 11:14 PM,  <maitysanchayan@gmail.com> 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 <maitysanchayan@gmail.com>
> >> > ---
> >> >  .../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
Sanchayan May 27, 2016, 6:33 a.m. UTC | #5
Hello Rob,

On 16-05-24 12:09:41, Rob Herring wrote:
> On Mon, May 23, 2016 at 11:14 PM,  <maitysanchayan@gmail.com> 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 <maitysanchayan@gmail.com>
> >> > ---
> >> >  .../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.
> 
> > 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. 

So if I understand correctly, the binding at the SoC level is fine.
Keeping that but removing the additional made-up properties, viz. below

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"

would be fine?

We would have something similar to here
http://www.spinics.net/lists/devicetree/msg80655.html

but now with the DT binding under SoC bus.

Regards,
Sanchayan.

> 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.
> 
> Rob
Arnd Bergmann May 27, 2016, 8:31 a.m. UTC | #6
On Friday, May 27, 2016 12:03:01 PM CEST maitysanchayan@gmail.com wrote:
> 
> So if I understand correctly, the binding at the SoC level is fine.
> Keeping that but removing the additional made-up properties, viz. below
> 
> 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"
> 
> would be fine?
> 
> We would have something similar to here
> http://www.spinics.net/lists/devicetree/msg80655.html
> 
> but now with the DT binding under SoC bus.
> 


You look up the OTP device as a syscon here, which seems odd since there
is already an nvmem driver for it. Shouldn't you use the nvmem API for
that?

	Arnd
Sanchayan May 27, 2016, 10:08 a.m. UTC | #7
On 16-05-27 10:31:55, Arnd Bergmann wrote:
> On Friday, May 27, 2016 12:03:01 PM CEST maitysanchayan@gmail.com wrote:
> > 
> > So if I understand correctly, the binding at the SoC level is fine.
> > Keeping that but removing the additional made-up properties, viz. below
> > 
> > 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"
> > 
> > would be fine?
> > 
> > We would have something similar to here
> > http://www.spinics.net/lists/devicetree/msg80655.html
> > 
> > but now with the DT binding under SoC bus.
> > 
> 
> 
> You look up the OTP device as a syscon here, which seems odd since there
> is already an nvmem driver for it. Shouldn't you use the nvmem API for
> that?
> 
> 	Arnd

I need the following 

nvmem-cells: phandles to two OCOTP child nodes ocotp_cfg0 and ocotp_cfg1
nvmem-cell-names: should contain string names "cfg0" and "cfg1"

to be able to use the NVMEM consumer API.

If I can put them at SoC node level then I certainly can use the NVMEM API.

Regards,
Sanchayan.
Stefan Agner May 27, 2016, 5:28 p.m. UTC | #8
On 2016-05-27 03:08, maitysanchayan@gmail.com wrote:
> On 16-05-27 10:31:55, Arnd Bergmann wrote:
>> On Friday, May 27, 2016 12:03:01 PM CEST maitysanchayan@gmail.com wrote:
>> >
>> > So if I understand correctly, the binding at the SoC level is fine.
>> > Keeping that but removing the additional made-up properties, viz. below
>> >
>> > 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"
>> >
>> > would be fine?
>> >
>> > We would have something similar to here
>> > http://www.spinics.net/lists/devicetree/msg80655.html
>> >
>> > but now with the DT binding under SoC bus.
>> >
>>
>>
>> You look up the OTP device as a syscon here, which seems odd since there
>> is already an nvmem driver for it. Shouldn't you use the nvmem API for
>> that?
>>
>> 	Arnd
> 
> I need the following 
> 
> nvmem-cells: phandles to two OCOTP child nodes ocotp_cfg0 and ocotp_cfg1
> nvmem-cell-names: should contain string names "cfg0" and "cfg1"
> 
> to be able to use the NVMEM consumer API.

I did not tested it, but it seems the NVMEM consumer API has some kind
of non-DT fallback:
http://lxr.free-electrons.com/source/drivers/nvmem/core.c#L827

Right now, it seems to me that it does not handle the case where we have
a of_node but don't want to use it... You might need to add some extra
handling if there is a of_node without nvmem-cell-names/nvmem-cells, and
fall back to nvmem_cell_get_from_list.

--
Stefan
Sanchayan May 27, 2016, 5:56 p.m. UTC | #9
Hello Stefan,

On 16-05-27 10:28:45, Stefan Agner wrote:
> On 2016-05-27 03:08, maitysanchayan@gmail.com wrote:
> > On 16-05-27 10:31:55, Arnd Bergmann wrote:
> >> On Friday, May 27, 2016 12:03:01 PM CEST maitysanchayan@gmail.com wrote:
> >> >
> >> > So if I understand correctly, the binding at the SoC level is fine.
> >> > Keeping that but removing the additional made-up properties, viz. below
> >> >
> >> > 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"
> >> >
> >> > would be fine?
> >> >
> >> > We would have something similar to here
> >> > http://www.spinics.net/lists/devicetree/msg80655.html
> >> >
> >> > but now with the DT binding under SoC bus.
> >> >
> >>
> >>
> >> You look up the OTP device as a syscon here, which seems odd since there
> >> is already an nvmem driver for it. Shouldn't you use the nvmem API for
> >> that?
> >>
> >> 	Arnd
> > 
> > I need the following 
> > 
> > nvmem-cells: phandles to two OCOTP child nodes ocotp_cfg0 and ocotp_cfg1
> > nvmem-cell-names: should contain string names "cfg0" and "cfg1"
> > 
> > to be able to use the NVMEM consumer API.
> 
> I did not tested it, but it seems the NVMEM consumer API has some kind
> of non-DT fallback:
> http://lxr.free-electrons.com/source/drivers/nvmem/core.c#L827
> 
> Right now, it seems to me that it does not handle the case where we have
> a of_node but don't want to use it... You might need to add some extra
> handling if there is a of_node without nvmem-cell-names/nvmem-cells, and
> fall back to nvmem_cell_get_from_list.
> 

I have already looked at the core nvmem code for options but wanted to avoid
any additons if possible.

However if adding the nvmem properties at the SoC node level is frowned upon
I will make the necessary changes required.

Regards,
Sanchayan.
Sanchayan June 9, 2016, 10:38 a.m. UTC | #10
Hello Rob,

On 16-05-27 15:38:17, maitysanchayan@gmail.com wrote:
> On 16-05-27 10:31:55, Arnd Bergmann wrote:
> > On Friday, May 27, 2016 12:03:01 PM CEST maitysanchayan@gmail.com wrote:
> > > 
> > > So if I understand correctly, the binding at the SoC level is fine.
> > > Keeping that but removing the additional made-up properties, viz. below
> > > 
> > > 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"
> > > 
> > > would be fine?
> > > 
> > > We would have something similar to here
> > > http://www.spinics.net/lists/devicetree/msg80655.html
> > > 
> > > but now with the DT binding under SoC bus.
> > > 
> > 
> > 
> > You look up the OTP device as a syscon here, which seems odd since there
> > is already an nvmem driver for it. Shouldn't you use the nvmem API for
> > that?
> > 
> > 	Arnd
> 
> I need the following 
> 
> nvmem-cells: phandles to two OCOTP child nodes ocotp_cfg0 and ocotp_cfg1
> nvmem-cell-names: should contain string names "cfg0" and "cfg1"
> 
> to be able to use the NVMEM consumer API.
> 
> If I can put them at SoC node level then I certainly can use the NVMEM API.
> 

Would the following be acceptable at the SoC node level

> nvmem-cells: phandles to two OCOTP child nodes ocotp_cfg0 and ocotp_cfg1
> nvmem-cell-names: should contain string names "cfg0" and "cfg1"

Regards,
Sanchayan.
diff mbox

Patch

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"
+
+Example:
+
+	vf610-soc {
+		compatible = "fsl,vf610-soc";
+		rom-revision = <&ocrom>;
+		mscm = <&mscm_cpucfg>;
+		nvmem-cells = <&ocotp_cfg0>, <&ocotp_cfg1>;
+		nvmem-cell-names = "cfg0", "cfg1";
+	};
diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
index cb58ef0..4410eb7 100644
--- a/drivers/soc/Kconfig
+++ b/drivers/soc/Kconfig
@@ -2,6 +2,7 @@  menu "SOC (System On Chip) specific Drivers"
 
 source "drivers/soc/bcm/Kconfig"
 source "drivers/soc/brcmstb/Kconfig"
+source "drivers/soc/fsl/Kconfig"
 source "drivers/soc/fsl/qe/Kconfig"
 source "drivers/soc/mediatek/Kconfig"
 source "drivers/soc/qcom/Kconfig"
diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig
new file mode 100644
index 0000000..029ea17
--- /dev/null
+++ b/drivers/soc/fsl/Kconfig
@@ -0,0 +1,10 @@ 
+#
+# Freescale SoC drivers
+
+config SOC_BUS_VF610
+	bool "SoC bus device for the Freescale Vybrid platform"
+	depends on NVMEM && NVMEM_VF610_OCOTP
+	select SOC_BUS
+	help
+	 Include support for the SoC bus on the Freescale Vybrid platform
+	 providing sysfs information about the module variant.
diff --git a/drivers/soc/fsl/Makefile b/drivers/soc/fsl/Makefile
index 203307f..afaf092 100644
--- a/drivers/soc/fsl/Makefile
+++ b/drivers/soc/fsl/Makefile
@@ -2,5 +2,6 @@ 
 # Makefile for the Linux Kernel SOC fsl specific device drivers
 #
 
+obj-$(CONFIG_SOC_VF610)			+= soc-vf610.o
 obj-$(CONFIG_QUICC_ENGINE)		+= qe/
 obj-$(CONFIG_CPM)			+= qe/
diff --git a/drivers/soc/fsl/soc-vf610.c b/drivers/soc/fsl/soc-vf610.c
new file mode 100644
index 0000000..571ba69
--- /dev/null
+++ b/drivers/soc/fsl/soc-vf610.c
@@ -0,0 +1,198 @@ 
+/*
+ * Copyright (C) 2016 Toradex AG.
+ *
+ * Author: Sanchayan Maity <sanchayan.maity@toradex.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/device.h>
+#include <linux/mfd/syscon.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/random.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/sys_soc.h>
+
+#define MSCM_CPxCOUNT_OFFSET	0x2C
+#define MSCM_CPxCFG1_OFFSET	0x14
+#define ROM_REVISION_OFFSET	0x80
+
+struct vf610_soc {
+	struct device *dev;
+	struct soc_device_attribute *soc_dev_attr;
+	struct soc_device *soc_dev;
+	struct nvmem_cell *ocotp_cfg0;
+	struct nvmem_cell *ocotp_cfg1;
+};
+
+static int vf610_soc_probe(struct platform_device *pdev)
+{
+	struct vf610_soc *info;
+	struct device *dev = &pdev->dev;
+	struct device_node *rom_node, *mscm_node;
+	struct regmap *rom_regmap, *mscm_regmap;
+	char soc_type[] = "xx0";
+	size_t id1_len, id2_len;
+	u32 cpucount, l2size, rom_rev;
+	u8 *socid1, *socid2;
+	int ret;
+
+	info = devm_kzalloc(dev, sizeof(struct vf610_soc), GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+	info->dev = dev;
+
+	info->ocotp_cfg0 = devm_nvmem_cell_get(dev, "cfg0");
+	if (IS_ERR(info->ocotp_cfg0))
+		return -EPROBE_DEFER;
+
+	info->ocotp_cfg1 = devm_nvmem_cell_get(dev, "cfg1");
+	if (IS_ERR(info->ocotp_cfg1))
+		return -EPROBE_DEFER;
+
+	socid1 = nvmem_cell_read(info->ocotp_cfg0, &id1_len);
+	if (IS_ERR(socid1)) {
+		dev_err(dev, "Could not read nvmem cell %ld\n",
+			PTR_ERR(socid1));
+		return PTR_ERR(socid1);
+	}
+
+	socid2 = nvmem_cell_read(info->ocotp_cfg1, &id2_len);
+	if (IS_ERR(socid2)) {
+		dev_err(dev, "Could not read nvmem cell %ld\n",
+			PTR_ERR(socid2));
+		ret = PTR_ERR(socid2);
+		goto out_socid2;
+	}
+	add_device_randomness(socid1, id1_len);
+	add_device_randomness(socid2, id2_len);
+
+	rom_node = of_parse_phandle(pdev->dev.of_node, "rom-revision", 0);
+	if (!rom_node) {
+		dev_err(dev, "Lookup failed for rom-revision node\n");
+		ret = -ENODEV;
+		goto out_rom_node;
+	}
+
+	mscm_node = of_parse_phandle(pdev->dev.of_node, "mscm", 0);
+	if (!mscm_node) {
+		dev_err(dev, "lookup failed for mscm node\n");
+		ret = -ENODEV;
+		goto out_mscm_node;
+	}
+
+	rom_regmap = syscon_node_to_regmap(rom_node);
+	if (IS_ERR(rom_regmap)) {
+		dev_err(dev, "regmap lookup for ocrom failed %ld\n",
+			PTR_ERR(rom_regmap));
+		ret = PTR_ERR(rom_regmap);
+		goto out;
+	}
+
+	mscm_regmap = syscon_node_to_regmap(mscm_node);
+	if (IS_ERR(mscm_regmap)) {
+		dev_err(dev, "regmap lookup for mscm failed %ld\n",
+			PTR_ERR(mscm_regmap));
+		ret = PTR_ERR(mscm_regmap);
+		goto out;
+	}
+
+	ret = regmap_read(rom_regmap, ROM_REVISION_OFFSET, &rom_rev);
+	if (ret) {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	ret = regmap_read(mscm_regmap, MSCM_CPxCOUNT_OFFSET, &cpucount);
+	if (ret) {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	ret = regmap_read(mscm_regmap, MSCM_CPxCFG1_OFFSET, &l2size);
+	if (ret) {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	soc_type[0] = cpucount ? '6' : '5'; /* Dual Core => VF6x0 */
+	soc_type[1] = l2size ? '1' : '0'; /* L2 Cache => VFx10 */
+
+	info->soc_dev_attr = devm_kzalloc(dev,
+				sizeof(info->soc_dev_attr), GFP_KERNEL);
+	if (!info->soc_dev_attr) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	info->soc_dev_attr->machine = devm_kasprintf(dev,
+					GFP_KERNEL, "Freescale Vybrid");
+	info->soc_dev_attr->soc_id = devm_kasprintf(dev,
+					GFP_KERNEL,
+					"%02x%02x%02x%02x%02x%02x%02x%02x",
+					socid1[3], socid1[2], socid1[1],
+					socid1[0], socid2[3], socid2[2],
+					socid2[1], socid2[0]);
+	info->soc_dev_attr->family = devm_kasprintf(&pdev->dev,
+					GFP_KERNEL, "Freescale Vybrid VF%s",
+					soc_type);
+	info->soc_dev_attr->revision = devm_kasprintf(dev,
+					GFP_KERNEL, "%08x", rom_rev);
+
+	platform_set_drvdata(pdev, info);
+
+	info->soc_dev = soc_device_register(info->soc_dev_attr);
+	if (IS_ERR(info->soc_dev)) {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	ret = 0;
+
+out:
+	of_node_put(mscm_node);
+out_mscm_node:
+	of_node_put(rom_node);
+out_rom_node:
+	kfree(socid2);
+out_socid2:
+	kfree(socid1);
+
+	return ret;
+}
+
+static int vf610_soc_remove(struct platform_device *pdev)
+{
+	struct vf610_soc *info = platform_get_drvdata(pdev);
+
+	if (info->soc_dev)
+		soc_device_unregister(info->soc_dev);
+
+	return 0;
+}
+
+static const struct of_device_id vf610_soc_match[] = {
+	{ .compatible = "fsl,vf610-soc", },
+	{ /* */ }
+};
+
+static struct platform_driver vf610_soc_driver = {
+	.probe		= vf610_soc_probe,
+	.remove		= vf610_soc_remove,
+	.driver		= {
+		.name = "vf610-soc",
+		.of_match_table = vf610_soc_match,
+	},
+};
+builtin_platform_driver(vf610_soc_driver);