RE: [PATCH v2 1/3] dt-binding: aspeed: Add LPC PCC controller

[Kevin Chen]

> On 04/03/2025 11:44, Kevin Chen wrote:
> > Add dt-bindings for Aspeed for Aspeed LPC POST code capture controller.
> 
> Please use subject prefixes matching the subsystem. You can get them for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory your
> patch is touching. For bindings, the preferred subjects are explained here:
> https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patch
> es.html#i-for-patch-submitters
> 
> Missing 's'.
> 
> >
> > Signed-off-by: Kevin Chen <kevin_chen@xxxxxxxxxxxxxx>
> > ---
> >  .../devicetree/bindings/mfd/aspeed-lpc.yaml   | 36
> +++++++++++++++++++
> >  1 file changed, 36 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/aspeed-lpc.yaml
> > b/Documentation/devicetree/bindings/mfd/aspeed-lpc.yaml
> > index 5dfe77aca167..367847bd7e75 100644
> > --- a/Documentation/devicetree/bindings/mfd/aspeed-lpc.yaml
> > +++ b/Documentation/devicetree/bindings/mfd/aspeed-lpc.yaml
> > @@ -149,6 +149,35 @@ patternProperties:
> >        - interrupts
> >        - snoop-ports
> >
> > +  "^lpc-pcc@[0-9a-f]+$":
> > +    type: object
> > +    additionalProperties: false
> > +
> > +    description:
> > +      The LPC pcc interface allows the BMC to listen on and record the
> data
> > +      bytes written by the Host to the targeted LPC I/O pots.
> > +
> > +    properties:
> > +      compatible:
> > +        items:
> > +          - enum:
> > +              - aspeed,ast2600-lpc-pcc
> > +
> > +      reg:
> > +        maxItems: 1
> > +
> > +      interrupts:
> > +        maxItems: 1
> > +
> > +      pcc-ports:
> 
> Missing vendor prefix
OK, so I need to add the vendor prefix such as "aspeed,pcc-ports". Right?
But I want to check the snoop-ports usage. In the HW controller, the usage should be the same.
Not only aspeed but nuvoton, there exist a HW module for the snoop-ports or pcc-ports usage.

> 
> > +        $ref: /schemas/types.yaml#/definitions/uint32-array
> > +        description: The LPC I/O ports to pcc
> 
> Description is too vague. Why would we encode I/O ports as some numbers
> instead of GPIOs for example? If these are ports, why this is not a graph?
For the port-mmaped I/O in x80 architecture, BMC need to handle specific port I/O in the relative HW module.
So, I need to add the pcc-ports property as the snoop-ports property in Documentation/devicetree/bindings/mfd/aspeed-lpc.yaml

> 
> Missing constraints - min/maxItems, defaults, minimum/maximum etc.
The port-mmaped I/O is defined from host, BMC as the device would capture the port I/O from the pcc-ports property defined in dts.

> 
> > +
> > +    required:
> > +      - compatible
> > +      - interrupts
> > +      - pcc-ports
> > +
> >    "^uart-routing@[0-9a-f]+$":
> >      $ref: /schemas/soc/aspeed/uart-routing.yaml#
> >      description: The UART routing control under LPC register space @@
> > -176,6 +205,13 @@ examples:
> >          #size-cells = <1>;
> >          ranges = <0x0 0x1e789000 0x1000>;
> >
> > +        lpc_pcc: lpc-pcc@0 {
> > +            compatible = "aspeed,ast2600-lpc-pcc";
> > +            reg = <0x0 0x140>;
> > +            interrupts = <GIC_SPI 145 IRQ_TYPE_LEVEL_HIGH>;
> > +            pcc-ports = <0x80>;
> 
> So what 0x80 stands for?
Host as x86 architecture would access the 0x80 port, which is mapped to the BMC PCC HW module.
As a result, x86 can keep the port-mmaped I/O usage and access the BMC device, which is needed to know which port using in the PCC module in BMC.

> 
> 
> Best regards,
> Krzysztof