RE: [v12, 5/8] soc: fsl: add GUTS driver for QorIQ platforms
Hi Scott, > -Original Message- > From: Scott Wood [mailto:o...@buserror.net] > Sent: Thursday, October 27, 2016 1:06 AM > To: Y.B. Lu; linux-...@vger.kernel.org; ulf.hans...@linaro.org; Arnd > Bergmann > Cc: linuxppc-dev@lists.ozlabs.org; devicet...@vger.kernel.org; linux-arm- > ker...@lists.infradead.org; linux-ker...@vger.kernel.org; linux- > c...@vger.kernel.org; linux-...@vger.kernel.org; iommu@lists.linux- > foundation.org; net...@vger.kernel.org; Mark Rutland; Rob Herring; > Russell King; Jochen Friedrich; Joerg Roedel; Claudiu Manoil; Bhupesh > Sharma; Qiang Zhao; Kumar Gala; Santosh Shilimkar; Leo Li; X.B. Xie; M.H. > Lian > Subject: Re: [v12, 5/8] soc: fsl: add GUTS driver for QorIQ platforms > > On Wed, 2016-09-21 at 14:57 +0800, Yangbo Lu wrote: > > diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig new > > file mode 100644 index 000..b99764c > > --- /dev/null > > +++ b/drivers/soc/fsl/Kconfig > > @@ -0,0 +1,19 @@ > > +# > > +# Freescale SOC drivers > > +# > > + > > +source "drivers/soc/fsl/qe/Kconfig" > > + > > +config FSL_GUTS > > + bool "Freescale QorIQ GUTS driver" > > + select SOC_BUS > > + help > > + The global utilities block controls power management, I/O device > > + enabling, power-onreset(POR) configuration monitoring, alternate > > + function selection for multiplexed signals,and clock control. > > + This driver is to manage and access global utilities block. > > + Initially only reading SVR and registering soc device are > > supported. > > + Other guts accesses, such as reading RCW, should eventually be > > moved > > + into this driver as well. > > + > > + If you want GUTS driver support, you should say Y here. > > This is user-enablable without dependencies, which means it will break > some randconfigs. If this is to be enabled via select then remove the > text after "bool". [Lu Yangbo-B47093] Will enable it via select and remove text after 'bool'. > > > +/* SoC die attribute definition for QorIQ platform */ static const > > +struct fsl_soc_die_attr fsl_soc_die[] = { #ifdef CONFIG_PPC > > + /* > > + * Power Architecture-based SoCs T Series > > + */ > > + > > + /* Die: T4240, SoC: T4240/T4160/T4080 */ > > + { .die = "T4240", > > + .svr = 0x8240, > > + .mask = 0xfff0, > > + }, > > + /* Die: T1040, SoC: T1040/T1020/T1042/T1022 */ > > + { .die = "T1040", > > + .svr = 0x8520, > > + .mask = 0xfff0, > > + }, > > + /* Die: T2080, SoC: T2080/T2081 */ > > + { .die = "T2080", > > + .svr = 0x8530, > > + .mask = 0xfff0, > > + }, > > + /* Die: T1024, SoC: T1024/T1014/T1023/T1013 */ > > + { .die = "T1024", > > + .svr = 0x8540, > > + .mask = 0xfff0, > > + }, > > +#endif /* CONFIG_PPC */ > > +#if defined(CONFIG_ARCH_MXC) || defined(CONFIG_ARCH_LAYERSCAPE) > > Will this driver ever be probed on MXC? Why do we need these ifdefs at > all? [Lu Yangbo-B47093] Will remove them. In the previous version, we use too many members for soc definition, so I add #ifdef for ARCH. CONFIG_ARCH_MXC was for ls1021a. > > > > + /* > > + * ARM-based SoCs LS Series > > + */ > > + > > + /* Die: LS1043A, SoC: LS1043A/LS1023A */ > > + { .die = "LS1043A", > > + .svr = 0x8792, > > + .mask = 0x, > > + }, > > + /* Die: LS2080A, SoC: LS2080A/LS2040A/LS2085A */ > > + { .die = "LS2080A", > > + .svr = 0x8701, > > + .mask = 0xff3f, > > + }, > > + /* Die: LS1088A, SoC: LS1088A/LS1048A/LS1084A/LS1044A */ > > + { .die = "LS1088A", > > + .svr = 0x8703, > > + .mask = 0xff3f, > > + }, > > + /* Die: LS1012A, SoC: LS1012A */ > > + { .die = "LS1012A", > > + .svr = 0x8704, > > + .mask = 0x, > > + }, > > + /* Die: LS1046A, SoC: LS1046A/LS1026A */ > > + { .die = "LS1046A", > > + .svr = 0x8707, > > + .mask = 0x, > > + }, > > + /* Die: LS2088A, SoC: LS2088A/LS2048A/LS2084A/LS2044A */ > > + { .die =
Re: [v12, 5/8] soc: fsl: add GUTS driver for QorIQ platforms
On Wed, 2016-09-21 at 14:57 +0800, Yangbo Lu wrote: > diff --git a/drivers/soc/fsl/Kconfig b/drivers/soc/fsl/Kconfig > new file mode 100644 > index 000..b99764c > --- /dev/null > +++ b/drivers/soc/fsl/Kconfig > @@ -0,0 +1,19 @@ > +# > +# Freescale SOC drivers > +# > + > +source "drivers/soc/fsl/qe/Kconfig" > + > +config FSL_GUTS > + bool "Freescale QorIQ GUTS driver" > + select SOC_BUS > + help > + The global utilities block controls power management, I/O device > + enabling, power-onreset(POR) configuration monitoring, alternate > + function selection for multiplexed signals,and clock control. > + This driver is to manage and access global utilities block. > + Initially only reading SVR and registering soc device are > supported. > + Other guts accesses, such as reading RCW, should eventually be > moved > + into this driver as well. > + > + If you want GUTS driver support, you should say Y here. This is user-enablable without dependencies, which means it will break some randconfigs. If this is to be enabled via select then remove the text after "bool". > +/* SoC die attribute definition for QorIQ platform */ > +static const struct fsl_soc_die_attr fsl_soc_die[] = { > +#ifdef CONFIG_PPC > + /* > + * Power Architecture-based SoCs T Series > + */ > + > + /* Die: T4240, SoC: T4240/T4160/T4080 */ > + { .die = "T4240", > + .svr = 0x8240, > + .mask = 0xfff0, > + }, > + /* Die: T1040, SoC: T1040/T1020/T1042/T1022 */ > + { .die = "T1040", > + .svr = 0x8520, > + .mask = 0xfff0, > + }, > + /* Die: T2080, SoC: T2080/T2081 */ > + { .die = "T2080", > + .svr = 0x8530, > + .mask = 0xfff0, > + }, > + /* Die: T1024, SoC: T1024/T1014/T1023/T1013 */ > + { .die = "T1024", > + .svr = 0x8540, > + .mask = 0xfff0, > + }, > +#endif /* CONFIG_PPC */ > +#if defined(CONFIG_ARCH_MXC) || defined(CONFIG_ARCH_LAYERSCAPE) Will this driver ever be probed on MXC? Why do we need these ifdefs at all? > + /* > + * ARM-based SoCs LS Series > + */ > + > + /* Die: LS1043A, SoC: LS1043A/LS1023A */ > + { .die = "LS1043A", > + .svr = 0x8792, > + .mask = 0x, > + }, > + /* Die: LS2080A, SoC: LS2080A/LS2040A/LS2085A */ > + { .die = "LS2080A", > + .svr = 0x8701, > + .mask = 0xff3f, > + }, > + /* Die: LS1088A, SoC: LS1088A/LS1048A/LS1084A/LS1044A */ > + { .die = "LS1088A", > + .svr = 0x8703, > + .mask = 0xff3f, > + }, > + /* Die: LS1012A, SoC: LS1012A */ > + { .die = "LS1012A", > + .svr = 0x8704, > + .mask = 0x, > + }, > + /* Die: LS1046A, SoC: LS1046A/LS1026A */ > + { .die = "LS1046A", > + .svr = 0x8707, > + .mask = 0x, > + }, > + /* Die: LS2088A, SoC: LS2088A/LS2048A/LS2084A/LS2044A */ > + { .die = "LS2088A", > + .svr = 0x8709, > + .mask = 0xff3f, > + }, > + /* Die: LS1021A, SoC: LS1021A/LS1020A/LS1022A > + * Note: Put this die at the end in cause of incorrect > identification > + */ > + { .die = "LS1021A", > + .svr = 0x8700, > + .mask = 0xfff0, > + }, > +#endif /* CONFIG_ARCH_MXC || CONFIG_ARCH_LAYERSCAPE */ Instead of relying on ordering, add more bits to the mask so that there's no overlap. I think 0xfff7 would work. > +out: > + kfree(soc_dev_attr.machine); > + kfree(soc_dev_attr.family); > + kfree(soc_dev_attr.soc_id); > + kfree(soc_dev_attr.revision); > + iounmap(guts->regs); > +out_free: > + kfree(guts); > + return ret; > +} Please use devm. -Scott