| Message ID | 20161117060633.3837-1-andrew@aj.id.au |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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>; +}; +
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