Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1524111 > unrolled thread

[RFC PATCH] mfd: dt: Add Aspeed LPC binding

Back to article view | Back to linux.kernel

Contents

  [RFC PATCH] mfd: dt: Add Aspeed LPC binding Andrew Jeffery <andrew@aj.id.au> - 2016-11-17 07:10 +0100
    Re: [RFC PATCH] mfd: dt: Add Aspeed LPC binding Arnd Bergmann <arnd@arndb.de> - 2016-11-17 10:20 +0100
      Re: [RFC PATCH] mfd: dt: Add Aspeed LPC binding Joel Stanley <joel@jms.id.au> - 2016-11-17 11:10 +0100
    Re: [RFC PATCH] mfd: dt: Add Aspeed LPC binding Linus Walleij <linus.walleij@linaro.org> - 2016-11-17 10:40 +0100
      Re: [RFC PATCH] mfd: dt: Add Aspeed LPC binding Joel Stanley <joel@jms.id.au> - 2016-11-17 11:30 +0100
      Re: [RFC PATCH] mfd: dt: Add Aspeed LPC binding Andrew Jeffery <andrew@aj.id.au> - 2016-11-18 00:50 +0100

#1524111 — [RFC PATCH] mfd: dt: Add Aspeed LPC binding

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---

I'd like to start a discussion about how to handle the LPC register space in
the Aspeed SoC. There are a number of issues, largely concerned with the layout
of the registers but also with the fact that LPC register state is used by the
pinmux to determine some pin functionality.

So the register layout is problematic. Registers for what I think are coherent
pieces of functionality functionality are littered through the layout: Post
Code Control registers (PCCR) are interleaved with LPC Host Controller
registers (LHCR) which are interleaved with Host Interface Controller registers
(HICR) which are segmented by LPC Snoop registers. It seems appropriate that
the whole thing is represented as an MFD syscon, with the alternative being
writing several distinct drivers that map some number of resources to access
all of their registers.

The disadvantage of not representing the LPC space as a syscon is the LPC Host
Controller driver will need to provide accessor functions for the pinmux
driver. Pinmux also depends on bits in the Display Controller, and would need
similar accessors provided there. The idea of using syscon regmaps removes the
need to write these accessors. If the changes between the AST2400 and AST2500
are anything to go by, the pinmux complexity of the SoCs will only increase
which will likely lead to the spread of these accessor functions.

Yet another option would be to only expose the LPC Host Controller as a syscon
instead of the whole LPC register space. I feel this is a little distasteful
as the LHCRs are not in contiguous memory space; as mentioned above they are
separated (seemingly randomly) by a PCCR. We could specify the LPC Host
Controller as a syscon and provide multiple resources: The syscon
implementation consumes the first resource which is what we desire here for use
with pinmux, but the driver would be left to map any subsequent resources. That
feels odd to me, but I'm interested in arguments for it.

We could also map the LPC Host Controller syscon across the offending PCCR, but
then any driver developed for the Post Code Controller would have to take a
reference to the LPC Host Controller regmap. I feel like we might wind up with
a syscon phandle spaghetti across the LPC controller if we use that approach.

Finally, the LPC registers can be divided in two: one set for H8S/2168
compatible LPC functionality, and the rest for Aspeed-specific registers.
Division in two is required if we are going to throw a regmap over the LPC
space as the H8S/2168 registers are 1-byte wide, whilst the Aspeed LPC
registers are 4-bytes. As far as I can tell we can treat them as separate
functionality without any loss; if there is a cross-over in configuration we
can have each phandle the other in the devicetree.

The final complication is the iBT device sits in the Aspeed-specific part of
the LPC controller and has an upstream driver that isn't regmap-capable.
Describing the LPC controller as a syscon will require some changes there, but
I think we can make it work without too much trouble.

What is the recommended approach to managing such hardware?

Cheers,

Andrew

PS: I sent a devicetree binding document out for the LPC Host Controller as
part of a recent pinmux series[1]. As it stands the example in the document
doesn't cover the all the registers relevant to the LPC Host Controller, which
was a motivation for trying to sort the problem out properly.

[1] https://www.spinics.net/lists/arm-kernel/msg540084.html

 .../devicetree/bindings/mfd/aspeed-lpc.txt         | 28 ++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/aspeed-lpc.txt

diff --git a/Documentation/devicetree/bindings/mfd/aspeed-lpc.txt b/Documentation/devicetree/bindings/mfd/aspeed-lpc.txt
new file mode 100644
index 000000000000..36c8a9e08dc6
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/aspeed-lpc.txt
@@ -0,0 +1,28 @@
+* Device tree bindings for the Aspeed LPC Controller
+
+The Aspeed LPC controller contains registers for a variety of functions. Not
+all registers for a function are contiguous, and some registers are referenced
+by functions outside the LPC controller.
+
+Note that this is separate from the H8S/2168 compatible register set occupying
+the start of the LPC controller address space.
+
+Some significant functions in the LPC controller:
+
+* LPC Host Controller
+* Host Interface Controller
+* iBT Controller
+* SuperIO Scratch registers
+
+Required properties:
+- compatible:		"aspeed,ast2500-lpc", "syscon"
+- reg:			contains offset/length value of the Aspeed-specific LPC
+			memory region.
+
+Example:
+
+lpc: lpc@1e7890a0 {
+	compatible = "aspeed,ast2500-lpc", "syscon";
+	reg = <0x1e789080 0x1e0>;
+};
+
-- 
2.9.3

[toc] | [next] | [standalone]

#1524179

On Thursday, November 17, 2016 4:36:33 PM CET Andrew Jeffery wrote:
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> ---
> 
> I'd like to start a discussion about how to handle the LPC register space in
> the Aspeed SoC. There are a number of issues, largely concerned with the layout
> of the registers but also with the fact that LPC register state is used by the
> pinmux to determine some pin functionality.
...
> 
> What is the recommended approach to managing such hardware?

Can you clarify which side of the LPC bus this is? We are currently having
a discussion for how to integrate the LPC master on an ARM64 server that
uses LPC to access an Aspeed LPC slave. For this one we want to use the
traditional ISA DT binding.

I'm guessing that you are interesting in the other side here, for mapping
the registers of the LPC slave on the Aspeed BMC, but that's not clear from
your email, as I'm assuming that the same chip has both master and slave
interfaces.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1524223

On Thu, Nov 17, 2016 at 7:46 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday, November 17, 2016 4:36:33 PM CET Andrew Jeffery wrote:
>> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
>> ---
>>
>> I'd like to start a discussion about how to handle the LPC register space in
>> the Aspeed SoC. There are a number of issues, largely concerned with the layout
>> of the registers but also with the fact that LPC register state is used by the
>> pinmux to determine some pin functionality.
> ...
>>
>> What is the recommended approach to managing such hardware?
>
> Can you clarify which side of the LPC bus this is? We are currently having
> a discussion for how to integrate the LPC master on an ARM64 server that
> uses LPC to access an Aspeed LPC slave. For this one we want to use the
> traditional ISA DT binding.

This is from the perspective of the BMC.

On the machines we are talking to, most (all?) access is performed
through the system firmware (skiboot).

> I'm guessing that you are interesting in the other side here, for mapping
> the registers of the LPC slave on the Aspeed BMC, but that's not clear from
> your email, as I'm assuming that the same chip has both master and slave
> interfaces.

Yep, we come from the "other side".

The BMC itself can operate the bus in Master or Slave mode. We are
interested in the slave case, for when the host is requesting access
to its system firmware at boot time.  This happens by mapping a region
of the BMC's AHB memory space into the LPC address space. After we
deal with pinmux, Andrew or I will be hacking on a driver to configure
that space, as the BMC needs to configure the window before the host
can boot. It's a pile of bits spread out over different parts of the
chip, and doesn't map nicely into any existing driver model we have in
the kernel.

Other functions include IPMI communication between the BMC and the
host via the LPC bus via the iBT interface. We have a driver for that
staged for 4.10. Then there's a mailbox, some "scratch" registers that
can be used by the firmware for whatever they see fit, and all kinds
of crazy legacy x86 stuff like POST code registers.

Cheers,

Joel

[toc] | [prev] | [next] | [standalone]

#1524192

On Thu, Nov 17, 2016 at 7:06 AM, Andrew Jeffery <andrew@aj.id.au> wrote:

> +* Device tree bindings for the Aspeed LPC Controller

We are going overboard with the lingo sometimes, to the point that we do not
understand how terse things become.

LPC = Low Pin Count, right?
Explain that right here: it is a slow external bus, right?

> +The Aspeed LPC controller contains registers for a variety of functions. Not
> +all registers for a function are contiguous, and some registers are referenced
> +by functions outside the LPC controller.
> +
> +Note that this is separate from the H8S/2168 compatible register set occupying
> +the start of the LPC controller address space.
> +
> +Some significant functions in the LPC controller:
> +
> +* LPC Host Controller
> +* Host Interface Controller

Host interface to what?

> +* iBT Controller

What is iBT?

> +* SuperIO Scratch registers

Again more context please.

With standards documents, either explain everything or provide
pointers for the information.

Yours,
Linus Walleij

[toc] | [prev] | [next] | [standalone]

#1524239

Hey Linus,

On Thu, Nov 17, 2016 at 8:00 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Thu, Nov 17, 2016 at 7:06 AM, Andrew Jeffery <andrew@aj.id.au> wrote:
>
>> +* Device tree bindings for the Aspeed LPC Controller
>
> We are going overboard with the lingo sometimes, to the point that we do not
> understand how terse things become.
>
> LPC = Low Pin Count, right?
> Explain that right here: it is a slow external bus, right?

Yep. 33MHz bus that generally connects a host CPU to other devices on
the same motherboard, such as management controllers.

https://en.wikipedia.org/wiki/Low_Pin_Count

Systems with Aspeed BMCs use the LPC to load the firmware from the BMC
into the host at boot, and send IPMI messages between the host and the
BMC.

BMC stands for Baseboard Management Controller. Back in the day it was
simple keyboard controller microcontroller, these days we use a 800MHz
ARM11 with half a gigabyte of RAM. It provides access to the boot
firmware for the host CPU, monitors the health of the system (fans,
temperature, error logging), and provides remote access for power
cycling and installation.

https://en.wikipedia.org/wiki/Intelligent_Platform_Management_Interface#Baseboard_management_controller

Andrew and I work on an open source project that is creating a new BMC
software stack called OpenBMC. You've been merging the wonderful
Aspeed pinmux patches that Andrew wrote recently, which was a major
bit of kernel work required for that project.

>> +The Aspeed LPC controller contains registers for a variety of functions. Not
>> +all registers for a function are contiguous, and some registers are referenced
>> +by functions outside the LPC controller.
>> +
>> +Note that this is separate from the H8S/2168 compatible register set occupying
>> +the start of the LPC controller address space.
>> +
>> +Some significant functions in the LPC controller:
>> +
>> +* LPC Host Controller
>> +* Host Interface Controller
>
> Host interface to what?

Host in this case is the CPU on the motherboard to do the heavy
lifting. x86, ARM64, PowerPC. So this is the BMC's interface to the
host.

>
>> +* iBT Controller
>
> What is iBT?

IPMI Byte Transfer? Something like that. It's the transport that the
host uses to send IPMI messages to the BMC and back.

The host side has been upstream for a while. The BMC side has a driver
staged for v4.10:

https://patchwork.ozlabs.org/patch/674973/

If you're really bored you can read up on IPMI from the specificaiton:

 http://www.intel.com/content/www/us/en/servers/ipmi/ipmi-second-gen-interface-spec-v2-rev1-1.html

tl;dr is it's a protocol for communication to the BMC. There is
in-band IPMI, which is between the host and the BMC over the eg. the
BT interface. Or out of band IPMI, which is where you use ipmitool
from your laptop to talk to the BMC over the network.

>
>> +* SuperIO Scratch registers
>
> Again more context please.

The SuperIO name is legacy x86 terminology. These are a few bytes
worth of data that can be set from one side and read by the other in
order to convey firmware-specific information. In OpenPower systems,
we use them to tell the host where to send it's console output when it
is booting.

> With standards documents, either explain everything or provide
> pointers for the information.

If only we could give you the pleasure of reading the Aspeed reference
manual. It's only available from Aspeed under NDA at this point in
time though. If you want further details on anything else then Andrew,
Ben or myself can explain.

Cheers,

Joel

[toc] | [prev] | [next] | [standalone]

#1524892

[Multipart message — attachments visible in raw view] — view raw

On Thu, 2016-11-17 at 10:30 +0100, Linus Walleij wrote:
> > On Thu, Nov 17, 2016 at 7:06 AM, Andrew Jeffery <andrew@aj.id.au> wrote:
> 
> > +* Device tree bindings for the Aspeed LPC Controller
> 
> We are going overboard with the lingo sometimes, to the point that we do not
> understand how terse things become.

Sorry for that, it is a bit terse. Sometimes I struggle to gauge how
much context I should provide.

Joel's reply covers the details of your queries below, but I'll make
sure to add the information to the bindings as well.

> 
> LPC = Low Pin Count, right?
> Explain that right here: it is a slow external bus, right?

> > +The Aspeed LPC controller contains registers for a variety of functions. Not
> > +all registers for a function are contiguous, and some registers are referenced
> > +by functions outside the LPC controller.
> > +
> > +Note that this is separate from the H8S/2168 compatible register set occupying
> > +the start of the LPC controller address space.
> > +
> > +Some significant functions in the LPC controller:
> > +
> > +* LPC Host Controller
> > +* Host Interface Controller
> 
> Host interface to what?
> 
> > +* iBT Controller
> 
> What is iBT?
> 
> > +* SuperIO Scratch registers
> 
> Again more context please.
> 
> With standards documents, either explain everything or provide
> pointers for the information.

ACK!

Andrew

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web