Re: [PATCH 1/3] i386: CS5535 chip support - cpu

[Ben Gardner <gardner.ben@gmail.com>]

Hi Jordan,

Sorry it took so long for me to respond.

On 12/7/05, Jordan Crouse <jordan.crouse@amd.com> 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