Configures the DIVIL component of the AMD CS5535 (Geode companion device).
This patch does the following:
- verifies the existence of the CS5535 by checking the DIVIL signature
- configures UART1 as a NS16550A
- (optionally) enables UART2 and configures it as a NS16550A
- (optionally) enables the SMBus/I2C interface
Signed-off-by: Ben Gardner <[email protected]>
On Wed, Dec 07, 2005 at 11:31:09AM -0600, Ben Gardner wrote:
>...
> --- linux-2.6.14.orig/arch/i386/kernel/Makefile
> +++ linux-2.6.14/arch/i386/kernel/Makefile
> @@ -42,6 +42,14 @@ EXTRA_AFLAGS := -traditional
>
> obj-$(CONFIG_SCx200) += scx200.o
>
> +obj-$(CONFIG_CS5535) += cs5535.o
> +ifeq ($(CONFIG_CS5535_SMB), y)
> +EXTRA_CFLAGS += -DCS5535_SMB
> +endif
> +ifeq ($(CONFIG_CS5535_UART2), y)
> +EXTRA_CFLAGS += -DCS5535_UART2
> +endif
>...
Please use CONFIG_CS5535_{SMB,UART2} in cs5535.c instead of setting
EXTRA_CFLAGS.
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
Configures the DIVIL component of the AMD CS5535 (Geode companion device).
This patch does the following:
- verifies the existence of the CS5535 by checking the DIVIL signature
- configures UART1 as a NS16550A
- (optionally) enables UART2 and configures it as a NS16550A
- (optionally) enables the SMBus/I2C interface
Signed-off-by: Ben Gardner <[email protected]>
---
Thanks for the feedback, Adrian.
Attached is a new patch with that error corrected.
Ben
On 12/7/05, Adrian Bunk <[email protected]> wrote:
> Please use CONFIG_CS5535_{SMB,UART2} in cs5535.c instead of setting
> EXTRA_CFLAGS.
Greetings Ben -
> - verifies the existence of the CS5535 by checking the DIVIL signature
> - configures UART1 as a NS16550A
I'm confused by all this. Not so much your code, which is correct, but
your reason for doing all of this in the first place. On a somewhat sane
BIOS, it should have already set up all your descriptors and GPIO pins
long before the kernel booted. Not that you should trust blindly the BIOS
in all things, but as long as this is handled for you, you might as well take
advantage of it.
Now, if your particular board has its own very good reasons for handling
this, then thats fine, but I don't think this should be accepted as CS5535
support, because it stands a fairly good chance of causing trouble over
the larger set of Geode devices. I'll certainly listen to arguments
to the contrary, though.
> - (optionally) enables UART2 and configures it as a NS16550A
This is similar to code that has previously been submitted, and is attached
below. Please review and comment.
> - (optionally) enables the SMBus/I2C interface
Is there any reason why you can't use the PCI devices instead? Granted,
the SMBUS and GPIO are part of a multi-function device (which has a higher
annoyance level), but it still seems better then passing around the global
variables that you read from the MSR. Also, PCI has the advantage of being
a much cleaner and well traveled path, which is always nice.
Regards,
Jordan
--
Jordan Crouse
Senior Linux Engineer
AMD - Personal Connectivity Solutions Group
<http://www.amd.com/embeddedprocessors>
GEODE: Support second UART on CS5535
The CS5535 has a muxed serial port that can either be used to drive GPIO pins
or a second 16550 UART. This code enables the UART via a command line option.
Signed-off-by: Jordan Crouse <[email protected]>
---
drivers/serial/Kconfig | 11 ++++++
drivers/serial/Makefile | 1 +
drivers/serial/cs5535_uart.c | 83 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 95 insertions(+), 0 deletions(-)
diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
index ad47c1b..0b1bb39 100644
--- a/drivers/serial/Kconfig
+++ b/drivers/serial/Kconfig
@@ -616,6 +616,17 @@ config SERIAL_AU1X00_CONSOLE
If you have an Alchemy AU1X00 processor (MIPS based) and you want
to use a console on a serial port, say Y. Otherwise, say N.
+config SERIAL_GEODE_UART2
+ bool "Enable AMD CS5535 UART2 as a serial port"
+ depends on MGEODE_LX
+ default y
+ select SERIAL_CORE
+ help
+ Select this to allow the user to select the secondary CS5535 UART
+ as a 16550 serial port instead of the default DDC interface. The
+ UART2 can be selected by specifying geodeuart2 on the command
+ line.
+
config SERIAL_CORE
tristate
diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
index d7c7c71..16dee50 100644
--- a/drivers/serial/Makefile
+++ b/drivers/serial/Makefile
@@ -57,3 +57,4 @@ obj-$(CONFIG_SERIAL_JSM) += jsm/
obj-$(CONFIG_SERIAL_TXX9) += serial_txx9.o
obj-$(CONFIG_SERIAL_VR41XX) += vr41xx_siu.o
obj-$(CONFIG_SERIAL_SGI_IOC4) += ioc4_serial.o
+obj-$(CONFIG_SERIAL_GEODE_UART2) += cs5535_uart.o
diff --git a/drivers/serial/cs5535_uart.c b/drivers/serial/cs5535_uart.c
new file mode 100644
index 0000000..d77c178
--- /dev/null
+++ b/drivers/serial/cs5535_uart.c
@@ -0,0 +1,83 @@
+/*
+ * Copyright (c) 2004-2005 Advanced Micro Devices, 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.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * The full GNU General Public License is included in this distribution in the
+ * file called COPYING
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <asm/msr.h>
+#include <asm/io.h>
+
+/* The CS5535 companion chip has two UARTs. This code enables the second
+ UART so other devices can use it. We do it here so we can expose the
+ port early enough for serial debugging
+*/
+
+/* Note - this does not check to see if the CS5535 actually exists */
+
+#define LO(b) (((1 << b) << 16) | 0x0000)
+#define HI(b) ((0x0000 << 16) | (1 << b))
+
+static u32 outtab[16] __initdata =
+{
+ 0x00,HI(4), 0x04,HI(4), 0x08,HI(4),
+ 0x0c,LO(4), 0x10,HI(4), 0x14,LO(4),
+ 0x18,LO(4), 0x1C,LO(4)
+};
+
+static u32 intab[16] __initdata = {
+ 0x20,HI(3), 0x24,LO(3), 0x28,LO(3),
+ 0x2C,LO(3), 0x34,HI(3), 0x38,LO(3),
+ 0x40,LO(3), 0x44,LO(3)
+};
+
+static int __init init_cs5535_uart2(char *str)
+{
+ u32 lo = 0, hi = 0;
+ u32 base; u32 i;
+
+ /* Enable UART2 instead of DDC */
+
+ rdmsr(0x51400014, lo, hi);
+ lo &= 0xFF8FFFFF;
+ lo |= 0x00500000; /* 0x2F8 ttyS1 */
+ wrmsr(0x51400014, lo, hi);
+
+ /* Set up the UART registers */
+ wrmsr(0x5140003E, 0x12, 0x00);
+
+ rdmsr(0x5140000C, lo, hi);
+ base = (u32)(lo & 0xFF00);
+
+ /* Enable the GPIO pins (in and out) */
+
+ for(i = 0; i < 16; i += 2) {
+ outl(outtab[i + 1], base + outtab[i]);
+ outl(intab[i + 1], base + intab[i]);
+ }
+
+ /* Enable the IRQ */
+
+ rdmsr(0x51400021,lo,hi);
+ lo &= 0x0FFFFFFF;
+ lo |= 0x30000000; /* IRQ 3 */
+ wrmsr(0x51400021,lo,hi);
+}
+
+__setup("geodeuart2", init_cs5535_uart2);
Hi Jordan,
Sorry it took so long for me to respond.
On 12/7/05, Jordan Crouse <[email protected]> wrote:
> Greetings Ben -
>
> > - verifies the existence of the CS5535 by checking the DIVIL signature
> > - configures UART1 as a NS16550A
>
> I'm confused by all this. Not so much your code, which is correct, but
> your reason for doing all of this in the first place. On a somewhat sane
> BIOS, it should have already set up all your descriptors and GPIO pins
> long before the kernel booted. Not that you should trust blindly the BIOS
> in all things, but as long as this is handled for you, you might as well take
> advantage of it.
I think your phrase "somewhat sane BIOS" sums up the problem nicely.
I threw in the UART1 config because UART2 has problems. Paranoia.
I'm not sure that the UART1 config is needed, so I'm willing to scrap
it, or wrap it in a config option.
If you are interested in my experience with UART2, see the kernel
buzilla report #5677.
> Now, if your particular board has its own very good reasons for handling
> this, then thats fine, but I don't think this should be accepted as CS5535
> support, because it stands a fairly good chance of causing trouble over
> the larger set of Geode devices. I'll certainly listen to arguments
> to the contrary, though.
I'm curious as to what trouble this could cause?
> > - (optionally) enables UART2 and configures it as a NS16550A
>
> This is similar to code that has previously been submitted, and is attached
> below. Please review and comment.
Your patch looks like it would also solve the UART2 problem.
What I still don't understand is why UART2 works fine in DOS and in
linux 2.6.13-rc6-mm1, but not in the current kernel - in short, why is
this needed?
My guess (not based on any data) is that a BIOS call (initiated by
Linux) uses the DDC, but doesn't restore the GPIO to its original
state. So, again, a BIOS problem.
Anyway, I recommend adding sanity checks on MSR_DIVIL_GLD_CAP and
MSR_LBAR_GPIO before writing to the IO port. The code as-is may cause
problems on a non-cs5535 board.
> > - (optionally) enables the SMBus/I2C interface
>
> Is there any reason why you can't use the PCI devices instead? Granted,
> the SMBUS and GPIO are part of a multi-function device (which has a higher
> annoyance level), but it still seems better then passing around the global
> variables that you read from the MSR. Also, PCI has the advantage of being
> a much cleaner and well traveled path, which is always nice.
I agree that PCI would work much cleaner.
I plan to revisit the GPIO & SMB drivers and get them to use PCI, but
that may not happen for a while.
> Regards,
> Jordan
Thanks for your input.
Ben
On 09/12/05 09:01 -0600, Ben Gardner wrote:
>
> I think your phrase "somewhat sane BIOS" sums up the problem nicely.
> I threw in the UART1 config because UART2 has problems. Paranoia.
> I'm not sure that the UART1 config is needed, so I'm willing to scrap
> it, or wrap it in a config option.
>
> If you are interested in my experience with UART2, see the kernel
> buzilla report #5677.
> My guess (not based on any data) is that a BIOS call (initiated by
> Linux) uses the DDC, but doesn't restore the GPIO to its original
> state. So, again, a BIOS problem.
Reading BZ 5677, I get some perspective on your issue now. I would agree
that the data sounds like something is subverting the GPIO pins for its
own use, and definately DDC is the logical conclusion.
Ok, so now with a bit of perspective on the issue, then I think that
adding code to win back the serial port is useful, but it should be wrapped
in a config or command line option, so that users can decide for themselves.
After all, DDC is useful too (if you're an X user).
> > Now, if your particular board has its own very good reasons for handling
> > this, then thats fine, but I don't think this should be accepted as CS5535
> > support, because it stands a fairly good chance of causing trouble over
> > the larger set of Geode devices. I'll certainly listen to arguments
> > to the contrary, though.
>
> I'm curious as to what trouble this could cause?
I'm mainly worried about resource conflicts and such, especially with the
first serial port. I'm less worried about the second port (as you could
see from my code), but on the particular platform where that
code was most useful, we had a fixed set of features so we knew we be more
carefree. Certainly, on the big development platforms with a full BIOS,
the code would be far more worriesome.
> Your patch looks like it would also solve the UART2 problem.
> What I still don't understand is why UART2 works fine in DOS and in
> linux 2.6.13-rc6-mm1, but not in the current kernel - in short, why is
> this needed?
Our internal BIOS defaults to DDC on the second CS5535/CS5536 serial
port, which normally isn't any big deal, because the first port is
available, and if worse comes to worse, I can always switch over to LPC.
But on platforms like the Thin Client RDK, which is more like a production
platform, there aren't any physical serial ports populated. So in order to
get access, our systems guys made a little dongle that plugged into
the VGA port and split out the DDC lines and turned them into a serial
connection. And this code was born to turn off the default DDC and
enable the serial port.
> Anyway, I recommend adding sanity checks on MSR_DIVIL_GLD_CAP and
> MSR_LBAR_GPIO before writing to the IO port. The code as-is may cause
> problems on a non-cs5535 board.
Thats a great point. Actually, we should check for both CS5535 and
CS5536, since the code will work on both.. :)
> Thanks for your input.
Thanks for your interest.
Jordan
--
Jordan Crouse
Senior Linux Engineer
AMD - Personal Connectivity Solutions Group
<http://www.amd.com/embeddedprocessors>
On Wed 07-12-05 12:42:26, Jordan Crouse wrote:
> +/* Note - this does not check to see if the CS5535 actually exists */
> +
> +#define LO(b) (((1 << b) << 16) | 0x0000)
> +#define HI(b) ((0x0000 << 16) | (1 << b))
> +
What kind of ninja mutant macros are these?
> +static u32 intab[16] __initdata = {
> + 0x20,HI(3), 0x24,LO(3), 0x28,LO(3),
> + 0x2C,LO(3), 0x34,HI(3), 0x38,LO(3),
> + 0x40,LO(3), 0x44,LO(3)
> +};
Can't you just use values here, without the macros?
Pavel
--
Thanks, Sharp!