2015-04-03 10:44:50

by Filip Brozović

[permalink] [raw]
Subject: [PATCH v2] powerpc/83xx: add support for mpc8306

Add chip specific initialization for the MPC8306.

Signed-off-by: Filip Brozovic <[email protected]>
---
Changes from v1:
- Fix multiplatform support for setting QE SNUMs
arch/powerpc/platforms/83xx/Kconfig | 8 ++++++++
arch/powerpc/platforms/83xx/mpc83xx.h | 4 ++++
arch/powerpc/platforms/83xx/usb.c | 14 +++++++++++---
arch/powerpc/sysdev/qe_lib/qe.c | 10 ++++++++--
drivers/gpio/Kconfig | 6 +++---
5 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/platforms/83xx/Kconfig b/arch/powerpc/platforms/83xx/Kconfig
index 2bdc8c8..904991d 100644
--- a/arch/powerpc/platforms/83xx/Kconfig
+++ b/arch/powerpc/platforms/83xx/Kconfig
@@ -113,6 +113,14 @@ config KMETER1

endif

+# used for gpio
+config PPC_MPC830x
+ bool
+ select ARCH_WANT_OPTIONAL_GPIOLIB
+
+config PPC_MPC8306
+ bool
+
# used for usb & gpio
config PPC_MPC831x
bool
diff --git a/arch/powerpc/platforms/83xx/mpc83xx.h b/arch/powerpc/platforms/83xx/mpc83xx.h
index 0cf74d7..4f21fb9 100644
--- a/arch/powerpc/platforms/83xx/mpc83xx.h
+++ b/arch/powerpc/platforms/83xx/mpc83xx.h
@@ -21,6 +21,8 @@

/* system i/o configuration register low */
#define MPC83XX_SICRL_OFFS 0x114
+#define MPC8306_SICRL_USB_MASK 0x003C0000
+#define MPC8306_SICRL_USB_ULPI 0x00000000
#define MPC834X_SICRL_USB_MASK 0x60000000
#define MPC834X_SICRL_USB0 0x20000000
#define MPC834X_SICRL_USB1 0x40000000
@@ -35,6 +37,8 @@

/* system i/o configuration register high */
#define MPC83XX_SICRH_OFFS 0x118
+#define MPC8306_SICRH_USB_MASK 0x0F00F300
+#define MPC8306_SICRH_USB_ULPI 0x00000000
#define MPC8308_SICRH_USB_MASK 0x000c0000
#define MPC8308_SICRH_USB_ULPI 0x00040000
#define MPC834X_SICRH_USB_UTMI 0x00020000
diff --git a/arch/powerpc/platforms/83xx/usb.c b/arch/powerpc/platforms/83xx/usb.c
index 5c31d82..a9db44b 100644
--- a/arch/powerpc/platforms/83xx/usb.c
+++ b/arch/powerpc/platforms/83xx/usb.c
@@ -99,7 +99,7 @@ int mpc834x_usb_cfg(void)
}
#endif /* CONFIG_PPC_MPC834x */

-#ifdef CONFIG_PPC_MPC831x
+#if defined(CONFIG_PPC_MPC8306) || defined(CONFIG_PPC_MPC831x)
int mpc831x_usb_cfg(void)
{
u32 temp;
@@ -128,6 +128,7 @@ int mpc831x_usb_cfg(void)
/* Configure clock */
immr_node = of_get_parent(np);
if (immr_node && (of_device_is_compatible(immr_node, "fsl,mpc8315-immr") ||
+ of_device_is_compatible(immr_node, "fsl,mpc8306-immr") ||
of_device_is_compatible(immr_node, "fsl,mpc8308-immr")))
clrsetbits_be32(immap + MPC83XX_SCCR_OFFS,
MPC8315_SCCR_USB_MASK,
@@ -139,7 +140,14 @@ int mpc831x_usb_cfg(void)

/* Configure pin mux for ULPI. There is no pin mux for UTMI */
if (prop && !strcmp(prop, "ulpi")) {
- if (of_device_is_compatible(immr_node, "fsl,mpc8308-immr")) {
+ if (of_device_is_compatible(immr_node, "fsl,mpc8306-immr")) {
+ clrsetbits_be32(immap + MPC83XX_SICRL_OFFS,
+ MPC8306_SICRL_USB_MASK,
+ MPC8306_SICRL_USB_ULPI);
+ clrsetbits_be32(immap + MPC83XX_SICRH_OFFS,
+ MPC8306_SICRH_USB_MASK,
+ MPC8306_SICRH_USB_ULPI);
+ } else if (of_device_is_compatible(immr_node, "fsl,mpc8308-immr")) {
clrsetbits_be32(immap + MPC83XX_SICRH_OFFS,
MPC8308_SICRH_USB_MASK,
MPC8308_SICRH_USB_ULPI);
@@ -210,7 +218,7 @@ out:
of_node_put(np);
return ret;
}
-#endif /* CONFIG_PPC_MPC831x */
+#endif /* CONFIG_PPC_MPC8306 || CONFIG_PPC_MPC831x */

#ifdef CONFIG_PPC_MPC837x
int mpc837x_usb_cfg(void)
diff --git a/arch/powerpc/sysdev/qe_lib/qe.c b/arch/powerpc/sysdev/qe_lib/qe.c
index c2518cd..f0c45f0 100644
--- a/arch/powerpc/sysdev/qe_lib/qe.c
+++ b/arch/powerpc/sysdev/qe_lib/qe.c
@@ -285,12 +285,18 @@ static void qe_snums_init(void)
0x28, 0x29, 0x38, 0x39, 0x48, 0x49, 0x58, 0x59,
0x68, 0x69, 0x78, 0x79, 0x80, 0x81,
};
+ static const u8 snum_init_14[] = {
+ 0x88, 0x89, 0x98, 0x99, 0xA8, 0xA9, 0xB8, 0xB9,
+ 0xC8, 0xC9, 0xD8, 0xD9, 0xE8, 0xE9,
+ };
static const u8 *snum_init;

qe_num_of_snum = qe_get_num_of_snums();

if (qe_num_of_snum == 76)
snum_init = snum_init_76;
+ else if (qe_num_of_snum == 14)
+ snum_init = snum_init_14;
else
snum_init = snum_init_46;

@@ -657,8 +663,8 @@ unsigned int qe_get_num_of_snums(void)
prop = of_get_property(qe, "fsl,qe-num-snums", &size);
if (prop && size == sizeof(*prop)) {
num_of_snums = *prop;
- if ((num_of_snums < 28) || (num_of_snums > QE_NUM_OF_SNUM)) {
- /* No QE ever has fewer than 28 SNUMs */
+ if ((num_of_snums < 14) || (num_of_snums > QE_NUM_OF_SNUM)) {
+ /* No QE ever has fewer than 14 SNUMs */
pr_err("QE: number of snum is invalid\n");
of_node_put(qe);
return -EINVAL;
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index c1e2ca3..4c60e7f 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -217,11 +217,11 @@ config GPIO_MPC5200

config GPIO_MPC8XXX
bool "MPC512x/MPC8xxx GPIO support"
- depends on PPC_MPC512x || PPC_MPC831x || PPC_MPC834x || PPC_MPC837x || \
- FSL_SOC_BOOKE || PPC_86xx
+ depends on PPC_MPC512x || PPC_MPC830x || PPC_MPC831x || PPC_MPC834x || \
+ PPC_MPC837x || FSL_SOC_BOOKE || PPC_86xx
help
Say Y here if you're going to use hardware that connects to the
- MPC512x/831x/834x/837x/8572/8610 GPIOs.
+ MPC512x/830x/831x/834x/837x/8572/8610 GPIOs.

config GPIO_MSM_V1
tristate "Qualcomm MSM GPIO v1"
--
2.1.4


2015-04-03 12:01:34

by Paul Bolle

[permalink] [raw]
Subject: Re: [PATCH v2] powerpc/83xx: add support for mpc8306

On Fri, 2015-04-03 at 12:44 +0200, Filip Brozovic wrote:
> --- a/arch/powerpc/platforms/83xx/Kconfig
> +++ b/arch/powerpc/platforms/83xx/Kconfig

> +# used for gpio
> +config PPC_MPC830x
> + bool
> + select ARCH_WANT_OPTIONAL_GPIOLIB
> +
> +config PPC_MPC8306
> + bool

To me these two new Kconfig symbols look pointless:
- they have no prompt, so one cannot set them manually;
- no other Kconfig symbol selects them;
- they do not default to 'y'.

I'm not aware of a way to set these symbols to 'y' outside of those
three. Is there perhaps a way for kconfig to set these symbols to 'y'
that I have missed?

Or do you expect to do one of these three things in a separate patch?

> --- a/arch/powerpc/platforms/83xx/usb.c
> +++ b/arch/powerpc/platforms/83xx/usb.c
> @@ -99,7 +99,7 @@ int mpc834x_usb_cfg(void)
> }
> #endif /* CONFIG_PPC_MPC834x */
>
> -#ifdef CONFIG_PPC_MPC831x
> +#if defined(CONFIG_PPC_MPC8306) || defined(CONFIG_PPC_MPC831x)
> int mpc831x_usb_cfg(void)
> {
> u32 temp;

So I think this hunk is pointless.

> @@ -210,7 +218,7 @@ out:
> of_node_put(np);
> return ret;
> }
> -#endif /* CONFIG_PPC_MPC831x */
> +#endif /* CONFIG_PPC_MPC8306 || CONFIG_PPC_MPC831x */
>
> #ifdef CONFIG_PPC_MPC837x
> int mpc837x_usb_cfg(void)

Ditto.

> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index c1e2ca3..4c60e7f 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -217,11 +217,11 @@ config GPIO_MPC5200
>
> config GPIO_MPC8XXX
> bool "MPC512x/MPC8xxx GPIO support"
> - depends on PPC_MPC512x || PPC_MPC831x || PPC_MPC834x || PPC_MPC837x || \
> - FSL_SOC_BOOKE || PPC_86xx
> + depends on PPC_MPC512x || PPC_MPC830x || PPC_MPC831x || PPC_MPC834x || \
> + PPC_MPC837x || FSL_SOC_BOOKE || PPC_86xx
> help
> Say Y here if you're going to use hardware that connects to the
> - MPC512x/831x/834x/837x/8572/8610 GPIOs.
> + MPC512x/830x/831x/834x/837x/8572/8610 GPIOs.
>
> config GPIO_MSM_V1
> tristate "Qualcomm MSM GPIO v1"

Ditto (except for the help change, which still might make sense).

Thanks,


Paul Bolle

2015-04-03 12:45:28

by Filip Brozović

[permalink] [raw]
Subject: Re: [PATCH v2] powerpc/83xx: add support for mpc8306

On 4/3/2015 2:01 PM, Paul Bolle wrote:
> On Fri, 2015-04-03 at 12:44 +0200, Filip Brozovic wrote:
>> --- a/arch/powerpc/platforms/83xx/Kconfig
>> +++ b/arch/powerpc/platforms/83xx/Kconfig
>
>> +# used for gpio
>> +config PPC_MPC830x
>> + bool
>> + select ARCH_WANT_OPTIONAL_GPIOLIB
>> +
>> +config PPC_MPC8306
>> + bool
>
> To me these two new Kconfig symbols look pointless:
> - they have no prompt, so one cannot set them manually;
> - no other Kconfig symbol selects them;
> - they do not default to 'y'.
>
> I'm not aware of a way to set these symbols to 'y' outside of those
> three. Is there perhaps a way for kconfig to set these symbols to 'y'
> that I have missed?
>
> Or do you expect to do one of these three things in a separate patch?
>

The idea was that boards in the Kconfig file would select these symbols
in order to enable support for the 8306. I mainly wanted to get this
patch into mainline in order to make kernel maintenance for a couple of
custom in-house developed boards easier. Since these boards are not
widely available and our customers are unlikely to want to change and
recompile the kernel, I have so far leaned towards not including support
for them in mainline. As far as I can see, boards which are included in
mainline right now are mostly evaluation boards which are easily
available at most electronics distributors.

That being said, I don't know what the "official" stance on this is; is
adding custom boards encouraged regardless of their availability (e.g.
if I develop a custom board with the intention of only ever actually
making a single prototype for personal use, should I go and submit
patches so that support makes it into the mainline kernel?), or should
there be a minimum level of public interest before incorporating custom
boards into mainline? If it's the latter, I suppose a solution would be
to include support for the Freescale MPC8306SOM in mainline. Of course,
this has its own problems, since someone would have to write and
maintain it (and I don't have an MPC8306SOM nor the time needed to do
maintenance).

- Filip

2015-04-03 20:24:22

by Scott Wood

[permalink] [raw]
Subject: Re: [PATCH v2] powerpc/83xx: add support for mpc8306

On Fri, 2015-04-03 at 14:45 +0200, Filip Brozović wrote:
> On 4/3/2015 2:01 PM, Paul Bolle wrote:
> > On Fri, 2015-04-03 at 12:44 +0200, Filip Brozovic wrote:
> >> --- a/arch/powerpc/platforms/83xx/Kconfig
> >> +++ b/arch/powerpc/platforms/83xx/Kconfig
> >
> >> +# used for gpio
> >> +config PPC_MPC830x
> >> + bool
> >> + select ARCH_WANT_OPTIONAL_GPIOLIB
> >> +
> >> +config PPC_MPC8306
> >> + bool
> >
> > To me these two new Kconfig symbols look pointless:
> > - they have no prompt, so one cannot set them manually;
> > - no other Kconfig symbol selects them;
> > - they do not default to 'y'.
> >
> > I'm not aware of a way to set these symbols to 'y' outside of those
> > three. Is there perhaps a way for kconfig to set these symbols to 'y'
> > that I have missed?
> >
> > Or do you expect to do one of these three things in a separate patch?
> >
>
> The idea was that boards in the Kconfig file would select these symbols
> in order to enable support for the 8306. I mainly wanted to get this
> patch into mainline in order to make kernel maintenance for a couple of
> custom in-house developed boards easier. Since these boards are not
> widely available and our customers are unlikely to want to change and
> recompile the kernel, I have so far leaned towards not including support
> for them in mainline. As far as I can see, boards which are included in
> mainline right now are mostly evaluation boards which are easily
> available at most electronics distributors.
>
> That being said, I don't know what the "official" stance on this is; is
> adding custom boards encouraged regardless of their availability (e.g.
> if I develop a custom board with the intention of only ever actually
> making a single prototype for personal use, should I go and submit
> patches so that support makes it into the mainline kernel?), or should
> there be a minimum level of public interest before incorporating custom
> boards into mainline? If it's the latter, I suppose a solution would be
> to include support for the Freescale MPC8306SOM in mainline. Of course,
> this has its own problems, since someone would have to write and
> maintain it (and I don't have an MPC8306SOM nor the time needed to do
> maintenance).

Custom boards are fine as long as someone will maintain them.

What are you using PPC_MPC8306 for in your custom board code?

-Scott

2015-04-03 21:31:09

by Paul Bolle

[permalink] [raw]
Subject: Re: [PATCH v2] powerpc/83xx: add support for mpc8306

On Fri, 2015-04-03 at 14:45 +0200, Filip Brozović wrote:
> The idea was that boards in the Kconfig file would select these symbols
> in order to enable support for the 8306. I mainly wanted to get this
> patch into mainline in order to make kernel maintenance for a couple of
> custom in-house developed boards easier.

The trouble with this patch is that, without some follow up patch, it
adds Kconfig symbols and preprocessor checks that are pointless for
mainline. I think we should not do that. (I'm also not sure how having
those bits in mainline substantially benefits out of tree development.)

But if you would put this in a series that also adds the code using
these bits, than I'd have no reason to object (and neither would, I
guess, the bots checking the tree for issues like that).

Thanks,


Paul Bolle

2015-04-08 12:24:49

by Filip Brozović

[permalink] [raw]
Subject: Re: [PATCH v2] powerpc/83xx: add support for mpc8306



On 4/3/2015 10:24 PM, Scott Wood wrote:
> What are you using PPC_MPC8306 for in your custom board code?

Sorry for the late reply, I was a bit busy over the Easter weekend.

I'm not currently using it for anything, so I guess I could remove it
and just use PPC_MPC830x in my board code. However, I think it's best if
this patch is ignored (for now), and I'll submit a patch series for the
8306 and a custom board in a couple of weeks/months, when I find some
time to clean up the board code.

- Filip