RE: [PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms
Hi Arnd, > -Original Message- > From: Arnd Bergmann [mailto:a...@arndb.de] > Sent: Thursday, July 07, 2016 4:30 PM > To: Yangbo Lu > Cc: Scott Wood; linuxppc-...@lists.ozlabs.org; Mark Rutland; Ulf Hansson; > linux-kernel@vger.kernel.org; linux-...@vger.kernel.org; linux- > c...@vger.kernel.org; Qiang Zhao; Russell King; Bhupesh Sharma; Joerg > Roedel; Claudiu Manoil; devicet...@vger.kernel.org; Kumar Gala; Rob > Herring; Santosh Shilimkar; linux-arm-ker...@lists.infradead.org; > net...@vger.kernel.org; linux-...@vger.kernel.org; Xiaobo Xie; Yang-Leo > Li; io...@lists.linux-foundation.org > Subject: Re: [PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms > > On Thursday, July 7, 2016 2:35:33 AM CEST Yangbo Lu wrote: > > Hi Arnd, > > > > Could you reply when you see the email? > > If your method doesn’t resolve the problem, we still want to use our > old patchset. > > > > This guts driver had been discussed about one year and blocked many > workaround upstream. > > So please help to review and comment soon. > > > > I don't really see how more discussion is going to help us here. I think > I've made it pretty clear that I don't want to see another platform > specific way to read an SoC revision and I've even sent a proof-of- > concept patch to show how the interface can work, now it's up to you to > fit the guts hardware into that and send a new patch series. > > Arnd I think your proof-of-concept patch is still in discussion. Some answers are needed from you to address Scott's comments on your patchset. Have you reached an agreement? Thanks. - Yangbo Lu
RE: [PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms
Hi Arnd, > -Original Message- > From: Arnd Bergmann [mailto:a...@arndb.de] > Sent: Thursday, July 07, 2016 4:30 PM > To: Yangbo Lu > Cc: Scott Wood; linuxppc-...@lists.ozlabs.org; Mark Rutland; Ulf Hansson; > linux-kernel@vger.kernel.org; linux-...@vger.kernel.org; linux- > c...@vger.kernel.org; Qiang Zhao; Russell King; Bhupesh Sharma; Joerg > Roedel; Claudiu Manoil; devicet...@vger.kernel.org; Kumar Gala; Rob > Herring; Santosh Shilimkar; linux-arm-ker...@lists.infradead.org; > net...@vger.kernel.org; linux-...@vger.kernel.org; Xiaobo Xie; Yang-Leo > Li; io...@lists.linux-foundation.org > Subject: Re: [PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms > > On Thursday, July 7, 2016 2:35:33 AM CEST Yangbo Lu wrote: > > Hi Arnd, > > > > Could you reply when you see the email? > > If your method doesn’t resolve the problem, we still want to use our > old patchset. > > > > This guts driver had been discussed about one year and blocked many > workaround upstream. > > So please help to review and comment soon. > > > > I don't really see how more discussion is going to help us here. I think > I've made it pretty clear that I don't want to see another platform > specific way to read an SoC revision and I've even sent a proof-of- > concept patch to show how the interface can work, now it's up to you to > fit the guts hardware into that and send a new patch series. > > Arnd I think your proof-of-concept patch is still in discussion. Some answers are needed from you to address Scott's comments on your patchset. Have you reached an agreement? Thanks. - Yangbo Lu
Re: [PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms
On Thu, 2016-07-07 at 10:30 +0200, Arnd Bergmann wrote: > On Thursday, July 7, 2016 2:35:33 AM CEST Yangbo Lu wrote: > > > > Hi Arnd, > > > > Could you reply when you see the email? > > If your method doesn’t resolve the problem, we still want to use our old > > patchset. > > > > This guts driver had been discussed about one year and blocked many > > workaround upstream. > > So please help to review and comment soon. > > > I don't really see how more discussion is going to help us here. I think > I've made it pretty clear that I don't want to see another platform > specific way to read an SoC revision and I've even sent a proof-of-concept > patch to show how the interface can work, now it's up to you to fit the > guts hardware into that and send a new patch series. In which relevant maintainership capacity are you NACKing it? -Scott
Re: [PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms
On Thu, 2016-07-07 at 10:30 +0200, Arnd Bergmann wrote: > On Thursday, July 7, 2016 2:35:33 AM CEST Yangbo Lu wrote: > > > > Hi Arnd, > > > > Could you reply when you see the email? > > If your method doesn’t resolve the problem, we still want to use our old > > patchset. > > > > This guts driver had been discussed about one year and blocked many > > workaround upstream. > > So please help to review and comment soon. > > > I don't really see how more discussion is going to help us here. I think > I've made it pretty clear that I don't want to see another platform > specific way to read an SoC revision and I've even sent a proof-of-concept > patch to show how the interface can work, now it's up to you to fit the > guts hardware into that and send a new patch series. In which relevant maintainership capacity are you NACKing it? -Scott
Re: [PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms
On Thursday, July 7, 2016 2:35:33 AM CEST Yangbo Lu wrote: > Hi Arnd, > > Could you reply when you see the email? > If your method doesn’t resolve the problem, we still want to use our old > patchset. > > This guts driver had been discussed about one year and blocked many > workaround upstream. > So please help to review and comment soon. > I don't really see how more discussion is going to help us here. I think I've made it pretty clear that I don't want to see another platform specific way to read an SoC revision and I've even sent a proof-of-concept patch to show how the interface can work, now it's up to you to fit the guts hardware into that and send a new patch series. Arnd
Re: [PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms
On Thursday, July 7, 2016 2:35:33 AM CEST Yangbo Lu wrote: > Hi Arnd, > > Could you reply when you see the email? > If your method doesn’t resolve the problem, we still want to use our old > patchset. > > This guts driver had been discussed about one year and blocked many > workaround upstream. > So please help to review and comment soon. > I don't really see how more discussion is going to help us here. I think I've made it pretty clear that I don't want to see another platform specific way to read an SoC revision and I've even sent a proof-of-concept patch to show how the interface can work, now it's up to you to fit the guts hardware into that and send a new patch series. Arnd
RE: [PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms
Hi Arnd,
Could you reply when you see the email?
If your method doesn’t resolve the problem, we still want to use our old
patchset.
This guts driver had been discussed about one year and blocked many workaround
upstream.
So please help to review and comment soon.
Thanks a lot.
Best regards,
Yangbo Lu
> -Original Message-
> From: Yangbo Lu
> Sent: Thursday, June 23, 2016 10:46 AM
> To: 'Scott Wood'; Arnd Bergmann; linuxppc-...@lists.ozlabs.org
> Cc: Mark Rutland; Ulf Hansson; linux-kernel@vger.kernel.org; linux-
> i...@vger.kernel.org; linux-...@vger.kernel.org; Qiang Zhao; Russell King;
> Bhupesh Sharma; Joerg Roedel; Claudiu Manoil; devicet...@vger.kernel.org;
> Kumar Gala; Rob Herring; Santosh Shilimkar; linux-arm-
> ker...@lists.infradead.org; net...@vger.kernel.org; linux-
> m...@vger.kernel.org; Xiaobo Xie; Yang-Leo Li; iommu@lists.linux-
> foundation.org
> Subject: RE: [PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms
>
> Hi Arnd,
>
> Could you comment on these?
> Thanks.
>
>
> Best regards,
> Yangbo Lu
>
>
> > -Original Message-
> > From: Scott Wood [mailto:o...@buserror.net]
> > Sent: Saturday, June 11, 2016 9:51 AM
> > To: Arnd Bergmann; linuxppc-...@lists.ozlabs.org
> > Cc: Mark Rutland; Ulf Hansson; linux-kernel@vger.kernel.org; linux-
> > i...@vger.kernel.org; linux-...@vger.kernel.org; Qiang Zhao; Russell
> > King; Bhupesh Sharma; Joerg Roedel; Claudiu Manoil;
> > devicet...@vger.kernel.org; Kumar Gala; Rob Herring; Santosh
> > Shilimkar; linux-arm- ker...@lists.infradead.org;
> > net...@vger.kernel.org; linux- m...@vger.kernel.org; Xiaobo Xie;
> > Yang-Leo Li; iommu@lists.linux- foundation.org; Yangbo Lu
> > Subject: Re: [PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms
> >
> > On Thu, 2016-06-02 at 10:43 +0200, Arnd Bergmann wrote:
> > > On Wednesday, June 1, 2016 8:47:22 PM CEST Scott Wood wrote:
> > > > On Mon, 2016-05-30 at 15:15 +0200, Arnd Bergmann wrote:
> > > > > diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c new
> > > > > file mode 100644 index ..2f30698f5bcf
> > > > > --- /dev/null
> > > > > +++ b/drivers/soc/fsl/guts.c
> > > > > @@ -0,0 +1,130 @@
> > > > > +/*
> > > > > + * Freescale QorIQ Platforms GUTS Driver
> > > > > + *
> > > > > + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> > > > > + *
> > > > > + * This program is free software; you can redistribute it
> > > > > +and/or modify
> > > > > + * it under the terms of the GNU General Public License as
> > > > > +published by
> > > > > + * the Free Software Foundation; either version 2 of the
> > > > > +License, or
> > > > > + * (at your option) any later version.
> > > > > + */
> > > > > +
> > > > > +#include
> > > > > +#include #include
> > > > > +#include #include #include
> > > > > + #include
> > > > > +
> > > > > +#define GUTS_PVR 0x0a0
> > > > > +#define GUTS_SVR 0x0a4
> > > > > +
> > > > > +struct guts {
> > > > > + void __iomem *regs;
> > > >
> > > > We already have a struct to define guts. Why are you not using it?
> > > > Why do you consider using it to be "abuse"? What if we want to
> > > > move more guts functionality into this driver?
> > >
> > > This structure was in the original patch, I left it in there, only
> > > removed the inclusion of the powerpc header file, which seemed to be
> > > misplaced.
> >
> > I'm not refering "struct guts". I'm referring to changing "struct
> > ccsr_guts __iomem *regs" into "void __iomem *regs".
> >
> > And it's not a powerpc header file.
> >
> > > > > +/*
> > > > > + * Table for matching compatible strings, for device tree
> > > > > + * guts node, for Freescale QorIQ SOCs.
> > > > > + */
> > > > > +static const struct of_device_id fsl_guts_of_match[] = {
> > > > > + /* For T4 & B4 Series SOCs */
> > > > > + { .compatible = "fsl,qoriq-device-config-1.0", .data = "T4/B4
> > > > > series" },
> > > > [snip]
> > > > > + { .compatible = "fsl,qoriq-device-config-2.0", .data = "P
> > > > > series"
> > > >
>
RE: [PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms
Hi Arnd,
Could you reply when you see the email?
If your method doesn’t resolve the problem, we still want to use our old
patchset.
This guts driver had been discussed about one year and blocked many workaround
upstream.
So please help to review and comment soon.
Thanks a lot.
Best regards,
Yangbo Lu
> -Original Message-
> From: Yangbo Lu
> Sent: Thursday, June 23, 2016 10:46 AM
> To: 'Scott Wood'; Arnd Bergmann; linuxppc-...@lists.ozlabs.org
> Cc: Mark Rutland; Ulf Hansson; linux-kernel@vger.kernel.org; linux-
> i...@vger.kernel.org; linux-...@vger.kernel.org; Qiang Zhao; Russell King;
> Bhupesh Sharma; Joerg Roedel; Claudiu Manoil; devicet...@vger.kernel.org;
> Kumar Gala; Rob Herring; Santosh Shilimkar; linux-arm-
> ker...@lists.infradead.org; net...@vger.kernel.org; linux-
> m...@vger.kernel.org; Xiaobo Xie; Yang-Leo Li; iommu@lists.linux-
> foundation.org
> Subject: RE: [PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms
>
> Hi Arnd,
>
> Could you comment on these?
> Thanks.
>
>
> Best regards,
> Yangbo Lu
>
>
> > -Original Message-
> > From: Scott Wood [mailto:o...@buserror.net]
> > Sent: Saturday, June 11, 2016 9:51 AM
> > To: Arnd Bergmann; linuxppc-...@lists.ozlabs.org
> > Cc: Mark Rutland; Ulf Hansson; linux-kernel@vger.kernel.org; linux-
> > i...@vger.kernel.org; linux-...@vger.kernel.org; Qiang Zhao; Russell
> > King; Bhupesh Sharma; Joerg Roedel; Claudiu Manoil;
> > devicet...@vger.kernel.org; Kumar Gala; Rob Herring; Santosh
> > Shilimkar; linux-arm- ker...@lists.infradead.org;
> > net...@vger.kernel.org; linux- m...@vger.kernel.org; Xiaobo Xie;
> > Yang-Leo Li; iommu@lists.linux- foundation.org; Yangbo Lu
> > Subject: Re: [PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms
> >
> > On Thu, 2016-06-02 at 10:43 +0200, Arnd Bergmann wrote:
> > > On Wednesday, June 1, 2016 8:47:22 PM CEST Scott Wood wrote:
> > > > On Mon, 2016-05-30 at 15:15 +0200, Arnd Bergmann wrote:
> > > > > diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c new
> > > > > file mode 100644 index ..2f30698f5bcf
> > > > > --- /dev/null
> > > > > +++ b/drivers/soc/fsl/guts.c
> > > > > @@ -0,0 +1,130 @@
> > > > > +/*
> > > > > + * Freescale QorIQ Platforms GUTS Driver
> > > > > + *
> > > > > + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> > > > > + *
> > > > > + * This program is free software; you can redistribute it
> > > > > +and/or modify
> > > > > + * it under the terms of the GNU General Public License as
> > > > > +published by
> > > > > + * the Free Software Foundation; either version 2 of the
> > > > > +License, or
> > > > > + * (at your option) any later version.
> > > > > + */
> > > > > +
> > > > > +#include
> > > > > +#include #include
> > > > > +#include #include #include
> > > > > + #include
> > > > > +
> > > > > +#define GUTS_PVR 0x0a0
> > > > > +#define GUTS_SVR 0x0a4
> > > > > +
> > > > > +struct guts {
> > > > > + void __iomem *regs;
> > > >
> > > > We already have a struct to define guts. Why are you not using it?
> > > > Why do you consider using it to be "abuse"? What if we want to
> > > > move more guts functionality into this driver?
> > >
> > > This structure was in the original patch, I left it in there, only
> > > removed the inclusion of the powerpc header file, which seemed to be
> > > misplaced.
> >
> > I'm not refering "struct guts". I'm referring to changing "struct
> > ccsr_guts __iomem *regs" into "void __iomem *regs".
> >
> > And it's not a powerpc header file.
> >
> > > > > +/*
> > > > > + * Table for matching compatible strings, for device tree
> > > > > + * guts node, for Freescale QorIQ SOCs.
> > > > > + */
> > > > > +static const struct of_device_id fsl_guts_of_match[] = {
> > > > > + /* For T4 & B4 Series SOCs */
> > > > > + { .compatible = "fsl,qoriq-device-config-1.0", .data = "T4/B4
> > > > > series" },
> > > > [snip]
> > > > > + { .compatible = "fsl,qoriq-device-config-2.0", .data = "P
> > > > > series"
> > > >
>
RE: [PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms
Hi Arnd,
Could you comment on these?
Thanks.
Best regards,
Yangbo Lu
> -Original Message-
> From: Scott Wood [mailto:o...@buserror.net]
> Sent: Saturday, June 11, 2016 9:51 AM
> To: Arnd Bergmann; linuxppc-...@lists.ozlabs.org
> Cc: Mark Rutland; Ulf Hansson; linux-kernel@vger.kernel.org; linux-
> i...@vger.kernel.org; linux-...@vger.kernel.org; Qiang Zhao; Russell King;
> Bhupesh Sharma; Joerg Roedel; Claudiu Manoil; devicet...@vger.kernel.org;
> Kumar Gala; Rob Herring; Santosh Shilimkar; linux-arm-
> ker...@lists.infradead.org; net...@vger.kernel.org; linux-
> m...@vger.kernel.org; Xiaobo Xie; Yang-Leo Li; iommu@lists.linux-
> foundation.org; Yangbo Lu
> Subject: Re: [PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms
>
> On Thu, 2016-06-02 at 10:43 +0200, Arnd Bergmann wrote:
> > On Wednesday, June 1, 2016 8:47:22 PM CEST Scott Wood wrote:
> > > On Mon, 2016-05-30 at 15:15 +0200, Arnd Bergmann wrote:
> > > > diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c new
> > > > file mode 100644 index ..2f30698f5bcf
> > > > --- /dev/null
> > > > +++ b/drivers/soc/fsl/guts.c
> > > > @@ -0,0 +1,130 @@
> > > > +/*
> > > > + * Freescale QorIQ Platforms GUTS Driver
> > > > + *
> > > > + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> > > > + *
> > > > + * This program is free software; you can redistribute it and/or
> > > > +modify
> > > > + * it under the terms of the GNU General Public License as
> > > > +published by
> > > > + * the Free Software Foundation; either version 2 of the License,
> > > > +or
> > > > + * (at your option) any later version.
> > > > + */
> > > > +
> > > > +#include
> > > > +#include #include
> > > > +#include #include #include
> > > > + #include
> > > > +
> > > > +#define GUTS_PVR 0x0a0
> > > > +#define GUTS_SVR 0x0a4
> > > > +
> > > > +struct guts {
> > > > + void __iomem *regs;
> > >
> > > We already have a struct to define guts. Why are you not using it?
> > > Why do you consider using it to be "abuse"? What if we want to move
> > > more guts functionality into this driver?
> >
> > This structure was in the original patch, I left it in there, only
> > removed the inclusion of the powerpc header file, which seemed to be
> > misplaced.
>
> I'm not refering "struct guts". I'm referring to changing "struct
> ccsr_guts __iomem *regs" into "void __iomem *regs".
>
> And it's not a powerpc header file.
>
> > > > +/*
> > > > + * Table for matching compatible strings, for device tree
> > > > + * guts node, for Freescale QorIQ SOCs.
> > > > + */
> > > > +static const struct of_device_id fsl_guts_of_match[] = {
> > > > + /* For T4 & B4 Series SOCs */
> > > > + { .compatible = "fsl,qoriq-device-config-1.0", .data = "T4/B4
> > > > series" },
> > > [snip]
> > > > + { .compatible = "fsl,qoriq-device-config-2.0", .data = "P
> > > > series"
> > >
> > > As noted in my comment on patch 3/4, these descriptions are reversed.
> > >
> > > They're also incomplete. t2080 has device config 2.0. t1040 is
> > > described as
> > > 2.0 though it should probably be 2.1 (or better, drop the generic
> > > compatible altogether).
> >
> > Ok. Ideally I think we'd even look up the specific SoC names from the
> > SVC rather than the compatible string. I just didn't have a good list
> > for those to put in the driver.
>
> The list is in arch/powerpc/include/asm/mpc85xx.h but I don't know why we
> need to convert it to a string in the first place.
>
> >
> > > > + /*
> > > > +* syscon devices default to little-endian, but on powerpc we
> > > > have
> > > > +* existing device trees with big-endian maps and an absent
> > > > endianess
> > > > +* "big-property"
> > > > +*/
> > > > + if (!IS_ENABLED(CONFIG_POWERPC) &&
> > > > + !of_property_read_bool(dev->of_node, "big-endian"))
> > > > + guts->little_endian = true;
> > >
> > > This is not a syscon device (Yangbo's patch to add a guts node on
> > >
RE: [PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms
Hi Arnd,
Could you comment on these?
Thanks.
Best regards,
Yangbo Lu
> -Original Message-
> From: Scott Wood [mailto:o...@buserror.net]
> Sent: Saturday, June 11, 2016 9:51 AM
> To: Arnd Bergmann; linuxppc-...@lists.ozlabs.org
> Cc: Mark Rutland; Ulf Hansson; linux-kernel@vger.kernel.org; linux-
> i...@vger.kernel.org; linux-...@vger.kernel.org; Qiang Zhao; Russell King;
> Bhupesh Sharma; Joerg Roedel; Claudiu Manoil; devicet...@vger.kernel.org;
> Kumar Gala; Rob Herring; Santosh Shilimkar; linux-arm-
> ker...@lists.infradead.org; net...@vger.kernel.org; linux-
> m...@vger.kernel.org; Xiaobo Xie; Yang-Leo Li; iommu@lists.linux-
> foundation.org; Yangbo Lu
> Subject: Re: [PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms
>
> On Thu, 2016-06-02 at 10:43 +0200, Arnd Bergmann wrote:
> > On Wednesday, June 1, 2016 8:47:22 PM CEST Scott Wood wrote:
> > > On Mon, 2016-05-30 at 15:15 +0200, Arnd Bergmann wrote:
> > > > diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c new
> > > > file mode 100644 index ..2f30698f5bcf
> > > > --- /dev/null
> > > > +++ b/drivers/soc/fsl/guts.c
> > > > @@ -0,0 +1,130 @@
> > > > +/*
> > > > + * Freescale QorIQ Platforms GUTS Driver
> > > > + *
> > > > + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> > > > + *
> > > > + * This program is free software; you can redistribute it and/or
> > > > +modify
> > > > + * it under the terms of the GNU General Public License as
> > > > +published by
> > > > + * the Free Software Foundation; either version 2 of the License,
> > > > +or
> > > > + * (at your option) any later version.
> > > > + */
> > > > +
> > > > +#include
> > > > +#include #include
> > > > +#include #include #include
> > > > + #include
> > > > +
> > > > +#define GUTS_PVR 0x0a0
> > > > +#define GUTS_SVR 0x0a4
> > > > +
> > > > +struct guts {
> > > > + void __iomem *regs;
> > >
> > > We already have a struct to define guts. Why are you not using it?
> > > Why do you consider using it to be "abuse"? What if we want to move
> > > more guts functionality into this driver?
> >
> > This structure was in the original patch, I left it in there, only
> > removed the inclusion of the powerpc header file, which seemed to be
> > misplaced.
>
> I'm not refering "struct guts". I'm referring to changing "struct
> ccsr_guts __iomem *regs" into "void __iomem *regs".
>
> And it's not a powerpc header file.
>
> > > > +/*
> > > > + * Table for matching compatible strings, for device tree
> > > > + * guts node, for Freescale QorIQ SOCs.
> > > > + */
> > > > +static const struct of_device_id fsl_guts_of_match[] = {
> > > > + /* For T4 & B4 Series SOCs */
> > > > + { .compatible = "fsl,qoriq-device-config-1.0", .data = "T4/B4
> > > > series" },
> > > [snip]
> > > > + { .compatible = "fsl,qoriq-device-config-2.0", .data = "P
> > > > series"
> > >
> > > As noted in my comment on patch 3/4, these descriptions are reversed.
> > >
> > > They're also incomplete. t2080 has device config 2.0. t1040 is
> > > described as
> > > 2.0 though it should probably be 2.1 (or better, drop the generic
> > > compatible altogether).
> >
> > Ok. Ideally I think we'd even look up the specific SoC names from the
> > SVC rather than the compatible string. I just didn't have a good list
> > for those to put in the driver.
>
> The list is in arch/powerpc/include/asm/mpc85xx.h but I don't know why we
> need to convert it to a string in the first place.
>
> >
> > > > + /*
> > > > +* syscon devices default to little-endian, but on powerpc we
> > > > have
> > > > +* existing device trees with big-endian maps and an absent
> > > > endianess
> > > > +* "big-property"
> > > > +*/
> > > > + if (!IS_ENABLED(CONFIG_POWERPC) &&
> > > > + !of_property_read_bool(dev->of_node, "big-endian"))
> > > > + guts->little_endian = true;
> > >
> > > This is not a syscon device (Yangbo's patch to add a guts node on
> > >
Re: [PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms
On Thu, 2016-06-02 at 10:43 +0200, Arnd Bergmann wrote:
> On Wednesday, June 1, 2016 8:47:22 PM CEST Scott Wood wrote:
> > On Mon, 2016-05-30 at 15:15 +0200, Arnd Bergmann wrote:
> > > diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
> > > new file mode 100644
> > > index ..2f30698f5bcf
> > > --- /dev/null
> > > +++ b/drivers/soc/fsl/guts.c
> > > @@ -0,0 +1,130 @@
> > > +/*
> > > + * Freescale QorIQ Platforms GUTS Driver
> > > + *
> > > + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> > > + *
> > > + * This program is free software; you can redistribute it and/or modify
> > > + * it under the terms of the GNU General Public License as published by
> > > + * the Free Software Foundation; either version 2 of the License, or
> > > + * (at your option) any later version.
> > > + */
> > > +
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +
> > > +#define GUTS_PVR 0x0a0
> > > +#define GUTS_SVR 0x0a4
> > > +
> > > +struct guts {
> > > + void __iomem *regs;
> >
> > We already have a struct to define guts. Why are you not using it? Why
> > do
> > you consider using it to be "abuse"? What if we want to move more guts
> > functionality into this driver?
>
> This structure was in the original patch, I left it in there, only
> removed the inclusion of the powerpc header file, which seemed to
> be misplaced.
I'm not refering "struct guts". I'm referring to changing
"struct ccsr_guts __iomem *regs" into "void __iomem *regs".
And it's not a powerpc header file.
> > > +/*
> > > + * Table for matching compatible strings, for device tree
> > > + * guts node, for Freescale QorIQ SOCs.
> > > + */
> > > +static const struct of_device_id fsl_guts_of_match[] = {
> > > + /* For T4 & B4 Series SOCs */
> > > + { .compatible = "fsl,qoriq-device-config-1.0", .data = "T4/B4
> > > series" },
> > [snip]
> > > + { .compatible = "fsl,qoriq-device-config-2.0", .data = "P
> > > series"
> >
> > As noted in my comment on patch 3/4, these descriptions are reversed.
> >
> > They're also incomplete. t2080 has device config 2.0. t1040 is described
> > as
> > 2.0 though it should probably be 2.1 (or better, drop the generic
> > compatible
> > altogether).
>
> Ok. Ideally I think we'd even look up the specific SoC names from the
> SVC rather than the compatible string. I just didn't have a good list
> for those to put in the driver.
The list is in arch/powerpc/include/asm/mpc85xx.h but I don't know why we need
to convert it to a string in the first place.
>
> > > + /*
> > > + * syscon devices default to little-endian, but on powerpc we
> > > have
> > > + * existing device trees with big-endian maps and an absent
> > > endianess
> > > + * "big-property"
> > > + */
> > > + if (!IS_ENABLED(CONFIG_POWERPC) &&
> > > + !of_property_read_bool(dev->of_node, "big-endian"))
> > > + guts->little_endian = true;
> >
> > This is not a syscon device (Yangbo's patch to add a guts node on ls2080
> > is
> > the only guts node that says "syscon", and that was a leftover from
> > earlier
> > revisions and should probably be removed). Even if it were, where is it
> > documented that syscon defaults to little-endian?
>
> Documentation/devicetree/bindings/regmap/regmap.txt
>
> We had a little screwup here, basically regmap (and by consequence, syscon)
> always defaulted to little-endian way before that was documented, so it's
> too late to change it,
What causes a device node to fall under the jurisdiction of regmap.txt?
Again, these nodes do not claim "syscon" compatibility.
> although I agree it would have made sense to document
> regmap to default to big-endian on powerpc.
Please don't. It's enough of a mess as is; no need to start throwing in
architecture ifdefs.
> > Documentation/devicetree/bindings/common-properties.txt says that the
> > individual binding specifies the default. The default for this node
> > should be
> > big-endian because that's what existed before there was a need to describe
> > the
> > endianness. And we need an update to the guts binding to specify that.
>
> Good point. This proably means that specifying both the "guts" and "syscon"
> compatible strings implies having to also specify the endianess explicitly
> both ways, because otherwise we break one of the two bindings.
Yes, but the node should only specify "guts".
>
> > > +
> > > + guts->regs = devm_ioremap_resource(dev, 0);
> > > + if (!guts->regs) {
> > > + ret = -ENOMEM;
> > > + kfree(guts);
> > > + goto out;
> > > + }
> > > +
> > > + fsl_guts_init(dev, guts);
> > > + ret = 0;
> > > +out:
> > > + return ret;
> > > +}
> > > +
> > > +static struct platform_driver fsl_soc_guts = {
> > > + .probe = fsl_guts_probe,
> > > + .driver.of_match_table = fsl_guts_of_match,
> > > +};
> > > +
> > > +module_platform_driver(fsl_soc_guts);
> >
> > Again, this means that the information is not available during early boot,
>
Re: [PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms
On Thu, 2016-06-02 at 10:43 +0200, Arnd Bergmann wrote:
> On Wednesday, June 1, 2016 8:47:22 PM CEST Scott Wood wrote:
> > On Mon, 2016-05-30 at 15:15 +0200, Arnd Bergmann wrote:
> > > diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
> > > new file mode 100644
> > > index ..2f30698f5bcf
> > > --- /dev/null
> > > +++ b/drivers/soc/fsl/guts.c
> > > @@ -0,0 +1,130 @@
> > > +/*
> > > + * Freescale QorIQ Platforms GUTS Driver
> > > + *
> > > + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> > > + *
> > > + * This program is free software; you can redistribute it and/or modify
> > > + * it under the terms of the GNU General Public License as published by
> > > + * the Free Software Foundation; either version 2 of the License, or
> > > + * (at your option) any later version.
> > > + */
> > > +
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +
> > > +#define GUTS_PVR 0x0a0
> > > +#define GUTS_SVR 0x0a4
> > > +
> > > +struct guts {
> > > + void __iomem *regs;
> >
> > We already have a struct to define guts. Why are you not using it? Why
> > do
> > you consider using it to be "abuse"? What if we want to move more guts
> > functionality into this driver?
>
> This structure was in the original patch, I left it in there, only
> removed the inclusion of the powerpc header file, which seemed to
> be misplaced.
I'm not refering "struct guts". I'm referring to changing
"struct ccsr_guts __iomem *regs" into "void __iomem *regs".
And it's not a powerpc header file.
> > > +/*
> > > + * Table for matching compatible strings, for device tree
> > > + * guts node, for Freescale QorIQ SOCs.
> > > + */
> > > +static const struct of_device_id fsl_guts_of_match[] = {
> > > + /* For T4 & B4 Series SOCs */
> > > + { .compatible = "fsl,qoriq-device-config-1.0", .data = "T4/B4
> > > series" },
> > [snip]
> > > + { .compatible = "fsl,qoriq-device-config-2.0", .data = "P
> > > series"
> >
> > As noted in my comment on patch 3/4, these descriptions are reversed.
> >
> > They're also incomplete. t2080 has device config 2.0. t1040 is described
> > as
> > 2.0 though it should probably be 2.1 (or better, drop the generic
> > compatible
> > altogether).
>
> Ok. Ideally I think we'd even look up the specific SoC names from the
> SVC rather than the compatible string. I just didn't have a good list
> for those to put in the driver.
The list is in arch/powerpc/include/asm/mpc85xx.h but I don't know why we need
to convert it to a string in the first place.
>
> > > + /*
> > > + * syscon devices default to little-endian, but on powerpc we
> > > have
> > > + * existing device trees with big-endian maps and an absent
> > > endianess
> > > + * "big-property"
> > > + */
> > > + if (!IS_ENABLED(CONFIG_POWERPC) &&
> > > + !of_property_read_bool(dev->of_node, "big-endian"))
> > > + guts->little_endian = true;
> >
> > This is not a syscon device (Yangbo's patch to add a guts node on ls2080
> > is
> > the only guts node that says "syscon", and that was a leftover from
> > earlier
> > revisions and should probably be removed). Even if it were, where is it
> > documented that syscon defaults to little-endian?
>
> Documentation/devicetree/bindings/regmap/regmap.txt
>
> We had a little screwup here, basically regmap (and by consequence, syscon)
> always defaulted to little-endian way before that was documented, so it's
> too late to change it,
What causes a device node to fall under the jurisdiction of regmap.txt?
Again, these nodes do not claim "syscon" compatibility.
> although I agree it would have made sense to document
> regmap to default to big-endian on powerpc.
Please don't. It's enough of a mess as is; no need to start throwing in
architecture ifdefs.
> > Documentation/devicetree/bindings/common-properties.txt says that the
> > individual binding specifies the default. The default for this node
> > should be
> > big-endian because that's what existed before there was a need to describe
> > the
> > endianness. And we need an update to the guts binding to specify that.
>
> Good point. This proably means that specifying both the "guts" and "syscon"
> compatible strings implies having to also specify the endianess explicitly
> both ways, because otherwise we break one of the two bindings.
Yes, but the node should only specify "guts".
>
> > > +
> > > + guts->regs = devm_ioremap_resource(dev, 0);
> > > + if (!guts->regs) {
> > > + ret = -ENOMEM;
> > > + kfree(guts);
> > > + goto out;
> > > + }
> > > +
> > > + fsl_guts_init(dev, guts);
> > > + ret = 0;
> > > +out:
> > > + return ret;
> > > +}
> > > +
> > > +static struct platform_driver fsl_soc_guts = {
> > > + .probe = fsl_guts_probe,
> > > + .driver.of_match_table = fsl_guts_of_match,
> > > +};
> > > +
> > > +module_platform_driver(fsl_soc_guts);
> >
> > Again, this means that the information is not available during early boot,
>
Re: [PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms
On Wednesday, June 1, 2016 8:47:22 PM CEST Scott Wood wrote:
> On Mon, 2016-05-30 at 15:15 +0200, Arnd Bergmann wrote:
> > diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
> > new file mode 100644
> > index ..2f30698f5bcf
> > --- /dev/null
> > +++ b/drivers/soc/fsl/guts.c
> > @@ -0,0 +1,130 @@
> > +/*
> > + * Freescale QorIQ Platforms GUTS Driver
> > + *
> > + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#define GUTS_PVR 0x0a0
> > +#define GUTS_SVR 0x0a4
> > +
> > +struct guts {
> > + void __iomem *regs;
>
> We already have a struct to define guts. Why are you not using it? Why do
> you consider using it to be "abuse"? What if we want to move more guts
> functionality into this driver?
This structure was in the original patch, I left it in there, only
removed the inclusion of the powerpc header file, which seemed to
be misplaced.
> > + bool little_endian;
> > + struct soc_device_attribute soc;
> > +};
> > +
> > +static u32 fsl_guts_get_svr(struct guts *guts)
> > +{
> > + if (guts->little_endian)
> > + return ioread32(guts->regs + GUTS_SVR);
> > + else
> > + return ioread32be(guts->regs + GUTS_SVR);
> > +}
> > +
> > +static u32 fsl_guts_get_pvr(struct guts *guts)
> > +{
> > + if (guts->little_endian)
> > + return ioread32(guts->regs + GUTS_PVR);
> > + else
> > + return ioread32be(guts->regs + GUTS_PVR);
> > +}
>
> You've removed the fallback to mfspr() on PPC, which would be helpful in some
> virtualized environments where we don't have the guts node (but do have other
> directly assigned devices). Of course, this is a consequence of the
> conversion into a platform device.
Right, it just didn't make any sense after the conversion. I couldn't
figure out what the intention was for the fallback. Virtualized environments
are an interesting case, but I'd also argue that a virtualized system is
not the original SoC anyway, so reporting the physical SoC as the
main system in the guest is a bit strange and we probably want to
come up with a different solution there.
In soc_device, we probably want to just report the type of hypervisor
in this case, which is what most users would expect there. For the
specific case of the mmc driver (are there any real use cases where
you'd pass on the mmc controller to a guest, as opposed to passing either
an emulated block device or the mmc device on an emulated mmc host?),
that means we have to come up with a different solution, but then
we'd be free to work around this by modifying the DT node of the mmc
device.
> > +/*
> > + * Table for matching compatible strings, for device tree
> > + * guts node, for Freescale QorIQ SOCs.
> > + */
> > +static const struct of_device_id fsl_guts_of_match[] = {
> > + /* For T4 & B4 Series SOCs */
> > + { .compatible = "fsl,qoriq-device-config-1.0", .data = "T4/B4
> > series" },
> [snip]
> > + { .compatible = "fsl,qoriq-device-config-2.0", .data = "P series"
>
> As noted in my comment on patch 3/4, these descriptions are reversed.
>
> They're also incomplete. t2080 has device config 2.0. t1040 is described as
> 2.0 though it should probably be 2.1 (or better, drop the generic compatible
> altogether).
Ok. Ideally I think we'd even look up the specific SoC names from the
SVC rather than the compatible string. I just didn't have a good list
for those to put in the driver.
> > + /*
> > +* syscon devices default to little-endian, but on powerpc we have
> > +* existing device trees with big-endian maps and an absent
> > endianess
> > +* "big-property"
> > +*/
> > + if (!IS_ENABLED(CONFIG_POWERPC) &&
> > + !of_property_read_bool(dev->of_node, "big-endian"))
> > + guts->little_endian = true;
>
> This is not a syscon device (Yangbo's patch to add a guts node on ls2080 is
> the only guts node that says "syscon", and that was a leftover from earlier
> revisions and should probably be removed). Even if it were, where is it
> documented that syscon defaults to little-endian?
Documentation/devicetree/bindings/regmap/regmap.txt
We had a little screwup here, basically regmap (and by consequence, syscon)
always defaulted to little-endian way before that was documented, so it's
too late to change it, although I agree it would have made sense to document
regmap to default to big-endian on powerpc.
> Documentation/devicetree/bindings/common-properties.txt says that the
> individual binding specifies the default. The default for this node should be
> big-endian because that's what existed before there was a
Re: [PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms
On Wednesday, June 1, 2016 8:47:22 PM CEST Scott Wood wrote:
> On Mon, 2016-05-30 at 15:15 +0200, Arnd Bergmann wrote:
> > diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
> > new file mode 100644
> > index ..2f30698f5bcf
> > --- /dev/null
> > +++ b/drivers/soc/fsl/guts.c
> > @@ -0,0 +1,130 @@
> > +/*
> > + * Freescale QorIQ Platforms GUTS Driver
> > + *
> > + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#define GUTS_PVR 0x0a0
> > +#define GUTS_SVR 0x0a4
> > +
> > +struct guts {
> > + void __iomem *regs;
>
> We already have a struct to define guts. Why are you not using it? Why do
> you consider using it to be "abuse"? What if we want to move more guts
> functionality into this driver?
This structure was in the original patch, I left it in there, only
removed the inclusion of the powerpc header file, which seemed to
be misplaced.
> > + bool little_endian;
> > + struct soc_device_attribute soc;
> > +};
> > +
> > +static u32 fsl_guts_get_svr(struct guts *guts)
> > +{
> > + if (guts->little_endian)
> > + return ioread32(guts->regs + GUTS_SVR);
> > + else
> > + return ioread32be(guts->regs + GUTS_SVR);
> > +}
> > +
> > +static u32 fsl_guts_get_pvr(struct guts *guts)
> > +{
> > + if (guts->little_endian)
> > + return ioread32(guts->regs + GUTS_PVR);
> > + else
> > + return ioread32be(guts->regs + GUTS_PVR);
> > +}
>
> You've removed the fallback to mfspr() on PPC, which would be helpful in some
> virtualized environments where we don't have the guts node (but do have other
> directly assigned devices). Of course, this is a consequence of the
> conversion into a platform device.
Right, it just didn't make any sense after the conversion. I couldn't
figure out what the intention was for the fallback. Virtualized environments
are an interesting case, but I'd also argue that a virtualized system is
not the original SoC anyway, so reporting the physical SoC as the
main system in the guest is a bit strange and we probably want to
come up with a different solution there.
In soc_device, we probably want to just report the type of hypervisor
in this case, which is what most users would expect there. For the
specific case of the mmc driver (are there any real use cases where
you'd pass on the mmc controller to a guest, as opposed to passing either
an emulated block device or the mmc device on an emulated mmc host?),
that means we have to come up with a different solution, but then
we'd be free to work around this by modifying the DT node of the mmc
device.
> > +/*
> > + * Table for matching compatible strings, for device tree
> > + * guts node, for Freescale QorIQ SOCs.
> > + */
> > +static const struct of_device_id fsl_guts_of_match[] = {
> > + /* For T4 & B4 Series SOCs */
> > + { .compatible = "fsl,qoriq-device-config-1.0", .data = "T4/B4
> > series" },
> [snip]
> > + { .compatible = "fsl,qoriq-device-config-2.0", .data = "P series"
>
> As noted in my comment on patch 3/4, these descriptions are reversed.
>
> They're also incomplete. t2080 has device config 2.0. t1040 is described as
> 2.0 though it should probably be 2.1 (or better, drop the generic compatible
> altogether).
Ok. Ideally I think we'd even look up the specific SoC names from the
SVC rather than the compatible string. I just didn't have a good list
for those to put in the driver.
> > + /*
> > +* syscon devices default to little-endian, but on powerpc we have
> > +* existing device trees with big-endian maps and an absent
> > endianess
> > +* "big-property"
> > +*/
> > + if (!IS_ENABLED(CONFIG_POWERPC) &&
> > + !of_property_read_bool(dev->of_node, "big-endian"))
> > + guts->little_endian = true;
>
> This is not a syscon device (Yangbo's patch to add a guts node on ls2080 is
> the only guts node that says "syscon", and that was a leftover from earlier
> revisions and should probably be removed). Even if it were, where is it
> documented that syscon defaults to little-endian?
Documentation/devicetree/bindings/regmap/regmap.txt
We had a little screwup here, basically regmap (and by consequence, syscon)
always defaulted to little-endian way before that was documented, so it's
too late to change it, although I agree it would have made sense to document
regmap to default to big-endian on powerpc.
> Documentation/devicetree/bindings/common-properties.txt says that the
> individual binding specifies the default. The default for this node should be
> big-endian because that's what existed before there was a
Re: [PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms
On Mon, 2016-05-30 at 15:15 +0200, Arnd Bergmann wrote:
> diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
> new file mode 100644
> index ..2f30698f5bcf
> --- /dev/null
> +++ b/drivers/soc/fsl/guts.c
> @@ -0,0 +1,130 @@
> +/*
> + * Freescale QorIQ Platforms GUTS Driver
> + *
> + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define GUTS_PVR 0x0a0
> +#define GUTS_SVR 0x0a4
> +
> +struct guts {
> + void __iomem *regs;
We already have a struct to define guts. Why are you not using it? Why do
you consider using it to be "abuse"? What if we want to move more guts
functionality into this driver?
> + bool little_endian;
> + struct soc_device_attribute soc;
> +};
> +
> +static u32 fsl_guts_get_svr(struct guts *guts)
> +{
> + if (guts->little_endian)
> + return ioread32(guts->regs + GUTS_SVR);
> + else
> + return ioread32be(guts->regs + GUTS_SVR);
> +}
> +
> +static u32 fsl_guts_get_pvr(struct guts *guts)
> +{
> + if (guts->little_endian)
> + return ioread32(guts->regs + GUTS_PVR);
> + else
> + return ioread32be(guts->regs + GUTS_PVR);
> +}
You've removed the fallback to mfspr() on PPC, which would be helpful in some
virtualized environments where we don't have the guts node (but do have other
directly assigned devices). Of course, this is a consequence of the
conversion into a platform device.
> +
> +/*
> + * Table for matching compatible strings, for device tree
> + * guts node, for Freescale QorIQ SOCs.
> + */
> +static const struct of_device_id fsl_guts_of_match[] = {
> + /* For T4 & B4 Series SOCs */
> + { .compatible = "fsl,qoriq-device-config-1.0", .data = "T4/B4
> series" },
[snip]
> + { .compatible = "fsl,qoriq-device-config-2.0", .data = "P series"
As noted in my comment on patch 3/4, these descriptions are reversed.
They're also incomplete. t2080 has device config 2.0. t1040 is described as
2.0 though it should probably be 2.1 (or better, drop the generic compatible
altogether).
> + /*
> + * syscon devices default to little-endian, but on powerpc we have
> + * existing device trees with big-endian maps and an absent
> endianess
> + * "big-property"
> + */
> + if (!IS_ENABLED(CONFIG_POWERPC) &&
> + !of_property_read_bool(dev->of_node, "big-endian"))
> + guts->little_endian = true;
This is not a syscon device (Yangbo's patch to add a guts node on ls2080 is
the only guts node that says "syscon", and that was a leftover from earlier
revisions and should probably be removed). Even if it were, where is it
documented that syscon defaults to little-endian?
Documentation/devicetree/bindings/common-properties.txt says that the
individual binding specifies the default. The default for this node should be
big-endian because that's what existed before there was a need to describe the
endianness. And we need an update to the guts binding to specify that.
> +
> + guts->regs = devm_ioremap_resource(dev, 0);
> + if (!guts->regs) {
> + ret = -ENOMEM;
> + kfree(guts);
> + goto out;
> + }
> +
> + fsl_guts_init(dev, guts);
> + ret = 0;
> +out:
> + return ret;
> +}
> +
> +static struct platform_driver fsl_soc_guts = {
> + .probe = fsl_guts_probe,
> + .driver.of_match_table = fsl_guts_of_match,
> +};
> +
> +module_platform_driver(fsl_soc_guts);
Again, this means that the information is not available during early boot,
such as in the clock driver. Thus we would not be able to convert clk-qoriq's
direct mfspr(SPRN_SVR) into an soc_device_match() (or anything else that makes
use of this file), nor would we be able to move its access of the guts RCW
registers into this driver.
-Scott
Re: [PATCH 2/4] soc: fsl: add GUTS driver for QorIQ platforms
On Mon, 2016-05-30 at 15:15 +0200, Arnd Bergmann wrote:
> diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
> new file mode 100644
> index ..2f30698f5bcf
> --- /dev/null
> +++ b/drivers/soc/fsl/guts.c
> @@ -0,0 +1,130 @@
> +/*
> + * Freescale QorIQ Platforms GUTS Driver
> + *
> + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define GUTS_PVR 0x0a0
> +#define GUTS_SVR 0x0a4
> +
> +struct guts {
> + void __iomem *regs;
We already have a struct to define guts. Why are you not using it? Why do
you consider using it to be "abuse"? What if we want to move more guts
functionality into this driver?
> + bool little_endian;
> + struct soc_device_attribute soc;
> +};
> +
> +static u32 fsl_guts_get_svr(struct guts *guts)
> +{
> + if (guts->little_endian)
> + return ioread32(guts->regs + GUTS_SVR);
> + else
> + return ioread32be(guts->regs + GUTS_SVR);
> +}
> +
> +static u32 fsl_guts_get_pvr(struct guts *guts)
> +{
> + if (guts->little_endian)
> + return ioread32(guts->regs + GUTS_PVR);
> + else
> + return ioread32be(guts->regs + GUTS_PVR);
> +}
You've removed the fallback to mfspr() on PPC, which would be helpful in some
virtualized environments where we don't have the guts node (but do have other
directly assigned devices). Of course, this is a consequence of the
conversion into a platform device.
> +
> +/*
> + * Table for matching compatible strings, for device tree
> + * guts node, for Freescale QorIQ SOCs.
> + */
> +static const struct of_device_id fsl_guts_of_match[] = {
> + /* For T4 & B4 Series SOCs */
> + { .compatible = "fsl,qoriq-device-config-1.0", .data = "T4/B4
> series" },
[snip]
> + { .compatible = "fsl,qoriq-device-config-2.0", .data = "P series"
As noted in my comment on patch 3/4, these descriptions are reversed.
They're also incomplete. t2080 has device config 2.0. t1040 is described as
2.0 though it should probably be 2.1 (or better, drop the generic compatible
altogether).
> + /*
> + * syscon devices default to little-endian, but on powerpc we have
> + * existing device trees with big-endian maps and an absent
> endianess
> + * "big-property"
> + */
> + if (!IS_ENABLED(CONFIG_POWERPC) &&
> + !of_property_read_bool(dev->of_node, "big-endian"))
> + guts->little_endian = true;
This is not a syscon device (Yangbo's patch to add a guts node on ls2080 is
the only guts node that says "syscon", and that was a leftover from earlier
revisions and should probably be removed). Even if it were, where is it
documented that syscon defaults to little-endian?
Documentation/devicetree/bindings/common-properties.txt says that the
individual binding specifies the default. The default for this node should be
big-endian because that's what existed before there was a need to describe the
endianness. And we need an update to the guts binding to specify that.
> +
> + guts->regs = devm_ioremap_resource(dev, 0);
> + if (!guts->regs) {
> + ret = -ENOMEM;
> + kfree(guts);
> + goto out;
> + }
> +
> + fsl_guts_init(dev, guts);
> + ret = 0;
> +out:
> + return ret;
> +}
> +
> +static struct platform_driver fsl_soc_guts = {
> + .probe = fsl_guts_probe,
> + .driver.of_match_table = fsl_guts_of_match,
> +};
> +
> +module_platform_driver(fsl_soc_guts);
Again, this means that the information is not available during early boot,
such as in the clock driver. Thus we would not be able to convert clk-qoriq's
direct mfspr(SPRN_SVR) into an soc_device_match() (or anything else that makes
use of this file), nor would we be able to move its access of the guts RCW
registers into this driver.
-Scott
