[PATCH v2 1/3] drivers/tty/serial/8250: Make Aspeed VUART SIRQ polarity configurable
Jeremy Kerr
jk at ozlabs.org
Thu Sep 5 11:14:44 AEST 2019
Hi Oskar,
Looks good to me, some minor comments though:
> +
> +What: /sys/bus/platform/drivers/aspeed-vuart/*/sirq_polarity
> +Date: July 2019
> +Contact: Oskar Senft <osk at google.com>
> +Description: Configures the polarity of the serial interrupt to the
> + host via the BMC LPC bus.
Can you mention what the value represents? 1/0 don't really indicate a
specific polarity.
Alternatively, we could use descriptive values (say, "active-low" /
"idle-low").
> @@ -310,6 +379,7 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
> struct resource *res;
> u32 clk, prop;
> int rc;
> + struct of_phandle_args espi_enabled_args;
Minor: can you reverse-christmas-tree this?
> @@ -402,6 +472,18 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
>
> vuart->line = rc;
>
> + rc = of_parse_phandle_with_fixed_args(
> + np, "espi-enabled", 2, 0, &espi_enabled_args);
> + if (rc < 0) {
> + dev_warn(&pdev->dev, "espi-enabled property not found\n");
In the binding spec, you've listed this property at optional, but here
we dev_warn() if its not present. Can we default to existing behaviour
if it's not there?
That may just be a matter of changing this to dev_debug.
Cheers,
Jeremy
More information about the openbmc
mailing list