This series generalizes the aspeed-vuart driver's device tree
properties to cover all the attributes it currently exposes via sysfs.
The aspeed,sirq-polarity-sense property was a bit of a design mistake
in that it ties Aspeed VUART SIRQ polarity to SCU register bits that
aren't really inherently related to it; the first patch in this series
deprecates it (though we hope to eventually remove it).
The rest of the series adds two new properties, aspeed,lpc-io-reg and
aspeed,lpc-interrupts. The latter allows describing the SIRQ polarity
(along with the interrupt number) directly, providing a simpler
replacement for aspeed,sirq-polarity-sense.
Changes since v4 [3]:
- fixed commit reference formatting in commit message
Changes since v3 [2]:
- renamed properties to match aspeed,ast2400-kcs-bmc
Changes since v2 [0]:
- expanded to also handle sirq number and lpc address in addition to
sirq polarity
- added default settings if DT properties not specified
- refactored existing sysfs code slightly, adding range checks
- cleaned up 'make dt_binding_check' warnings
Changes since v1 [1]:
- deprecate and retain aspeed,sirq-polarity-sense instead of removing it
- drop e3c246d4i dts addition from this series
[0] https://lore.kernel.org/openbmc/[email protected]/
[1] https://lore.kernel.org/openbmc/[email protected]/
[2] https://lore.kernel.org/openbmc/[email protected]/
[3] https://lore.kernel.org/openbmc/[email protected]/
Zev Weiss (4):
dt-bindings: serial: 8250: deprecate aspeed,sirq-polarity-sense
drivers/tty/serial/8250: refactor sirq and lpc address setting code
drivers/tty/serial/8250: add aspeed,lpc-io-reg and
aspeed,lpc-interrupts DT properties
dt-bindings: serial: 8250: add aspeed,lpc-io-reg and
aspeed,lpc-interrupts
.../devicetree/bindings/serial/8250.yaml | 28 +++++-
drivers/tty/serial/8250/8250_aspeed_vuart.c | 95 +++++++++++++++----
2 files changed, 103 insertions(+), 20 deletions(-)
--
2.31.1
This property ties SIRQ polarity to SCU register bits that don't
necessarily have any direct relationship to it; the only use of it was
removed in commit c82bf6e133d3 ("ARM: aspeed: g5: Do not set sirq
polarity").
Signed-off-by: Zev Weiss <[email protected]>
Reviewed-by: Joel Stanley <[email protected]>
---
Documentation/devicetree/bindings/serial/8250.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/serial/8250.yaml b/Documentation/devicetree/bindings/serial/8250.yaml
index f54cae9ff7b2..491b9297432d 100644
--- a/Documentation/devicetree/bindings/serial/8250.yaml
+++ b/Documentation/devicetree/bindings/serial/8250.yaml
@@ -188,6 +188,7 @@ properties:
offset and bit number to identify how the SIRQ polarity should be
configured. One possible data source is the LPC/eSPI mode bit. Only
applicable to aspeed,ast2500-vuart.
+ deprecated: true
required:
- reg
--
2.31.1
These allow describing all the Aspeed VUART attributes currently
available via sysfs. aspeed,sirq provides a replacement for the
deprecated aspeed,sirq-polarity-sense property.
Signed-off-by: Zev Weiss <[email protected]>
---
drivers/tty/serial/8250/8250_aspeed_vuart.c | 44 ++++++++++++++++++++-
1 file changed, 43 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c b/drivers/tty/serial/8250/8250_aspeed_vuart.c
index 8433f8dbb186..75ef006fa24b 100644
--- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
+++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
@@ -28,6 +28,10 @@
#define ASPEED_VUART_ADDRL 0x28
#define ASPEED_VUART_ADDRH 0x2c
+#define ASPEED_VUART_DEFAULT_LPC_ADDR 0x3f8
+#define ASPEED_VUART_DEFAULT_SIRQ 4
+#define ASPEED_VUART_DEFAULT_SIRQ_POLARITY IRQ_TYPE_LEVEL_LOW
+
struct aspeed_vuart {
struct device *dev;
void __iomem *regs;
@@ -393,7 +397,8 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
struct aspeed_vuart *vuart;
struct device_node *np;
struct resource *res;
- u32 clk, prop;
+ u32 clk, prop, sirq[2];
+ bool sirq_polarity;
int rc;
np = pdev->dev.of_node;
@@ -501,6 +506,43 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
of_node_put(sirq_polarity_sense_args.np);
}
+ rc = of_property_read_u32(np, "aspeed,lpc-io-reg", &prop);
+ if (rc < 0)
+ prop = ASPEED_VUART_DEFAULT_LPC_ADDR;
+
+ rc = aspeed_vuart_set_lpc_address(vuart, prop);
+ if (rc < 0) {
+ dev_err(&pdev->dev, "invalid value in aspeed,lpc-io-reg property\n");
+ goto err_clk_disable;
+ }
+
+ rc = of_property_read_u32_array(np, "aspeed,lpc-interrupts", sirq, 2);
+ if (rc < 0) {
+ sirq[0] = ASPEED_VUART_DEFAULT_SIRQ;
+ sirq[1] = ASPEED_VUART_DEFAULT_SIRQ_POLARITY;
+ }
+
+ rc = aspeed_vuart_set_sirq(vuart, sirq[0]);
+ if (rc < 0) {
+ dev_err(&pdev->dev, "invalid sirq number in aspeed,lpc-interrupts property\n");
+ goto err_clk_disable;
+ }
+
+ switch (sirq[1]) {
+ case IRQ_TYPE_LEVEL_LOW:
+ sirq_polarity = false;
+ break;
+ case IRQ_TYPE_LEVEL_HIGH:
+ sirq_polarity = true;
+ break;
+ default:
+ dev_err(&pdev->dev, "invalid sirq polarity in aspeed,lpc-interrupts property\n");
+ rc = -EINVAL;
+ goto err_clk_disable;
+ }
+
+ aspeed_vuart_set_sirq_polarity(vuart, sirq_polarity);
+
aspeed_vuart_set_enabled(vuart, true);
aspeed_vuart_set_host_tx_discard(vuart, true);
platform_set_drvdata(pdev, vuart);
--
2.31.1
On Wed, 07 Apr 2021 20:16:34 -0500, Zev Weiss wrote:
> This property ties SIRQ polarity to SCU register bits that don't
> necessarily have any direct relationship to it; the only use of it was
> removed in commit c82bf6e133d3 ("ARM: aspeed: g5: Do not set sirq
> polarity").
>
> Signed-off-by: Zev Weiss <[email protected]>
> Reviewed-by: Joel Stanley <[email protected]>
> ---
> Documentation/devicetree/bindings/serial/8250.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
Acked-by: Rob Herring <[email protected]>
Hi Zev,
A couple of minor comments:
On Thu, 8 Apr 2021, at 10:46, Zev Weiss wrote:
> These allow describing all the Aspeed VUART attributes currently
> available via sysfs. aspeed,sirq
aspeed,lpc-interrupts now
> provides a replacement for the
> deprecated aspeed,sirq-polarity-sense property.
>
> Signed-off-by: Zev Weiss <[email protected]>
> ---
> drivers/tty/serial/8250/8250_aspeed_vuart.c | 44 ++++++++++++++++++++-
> 1 file changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c
> b/drivers/tty/serial/8250/8250_aspeed_vuart.c
> index 8433f8dbb186..75ef006fa24b 100644
> --- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
> +++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
> @@ -28,6 +28,10 @@
> #define ASPEED_VUART_ADDRL 0x28
> #define ASPEED_VUART_ADDRH 0x2c
>
> +#define ASPEED_VUART_DEFAULT_LPC_ADDR 0x3f8
> +#define ASPEED_VUART_DEFAULT_SIRQ 4
> +#define ASPEED_VUART_DEFAULT_SIRQ_POLARITY IRQ_TYPE_LEVEL_LOW
> +
> struct aspeed_vuart {
> struct device *dev;
> void __iomem *regs;
> @@ -393,7 +397,8 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
> struct aspeed_vuart *vuart;
> struct device_node *np;
> struct resource *res;
> - u32 clk, prop;
> + u32 clk, prop, sirq[2];
> + bool sirq_polarity;
> int rc;
>
> np = pdev->dev.of_node;
> @@ -501,6 +506,43 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
> of_node_put(sirq_polarity_sense_args.np);
> }
>
> + rc = of_property_read_u32(np, "aspeed,lpc-io-reg", &prop);
> + if (rc < 0)
> + prop = ASPEED_VUART_DEFAULT_LPC_ADDR;
> +
> + rc = aspeed_vuart_set_lpc_address(vuart, prop);
> + if (rc < 0) {
> + dev_err(&pdev->dev, "invalid value in aspeed,lpc-io-reg property\n");
> + goto err_clk_disable;
> + }
> +
> + rc = of_property_read_u32_array(np, "aspeed,lpc-interrupts", sirq, 2);
> + if (rc < 0) {
> + sirq[0] = ASPEED_VUART_DEFAULT_SIRQ;
> + sirq[1] = ASPEED_VUART_DEFAULT_SIRQ_POLARITY;
> + }
> +
> + rc = aspeed_vuart_set_sirq(vuart, sirq[0]);
> + if (rc < 0) {
> + dev_err(&pdev->dev, "invalid sirq number in aspeed,lpc-interrupts > property\n");
> + goto err_clk_disable;
> + }
> +
> + switch (sirq[1]) {
> + case IRQ_TYPE_LEVEL_LOW:
> + sirq_polarity = false;
> + break;
> + case IRQ_TYPE_LEVEL_HIGH:
> + sirq_polarity = true;
> + break;
> + default:
> + dev_err(&pdev->dev, "invalid sirq polarity in aspeed,lpc-interrupts
> property\n");
> + rc = -EINVAL;
> + goto err_clk_disable;
> + }
A bit ugly open-coding the mapping and error handling, maybe worth a helper?
Looks okay otherwise.
Cheers,
Andrew
On Fri, Apr 09, 2021 at 12:14:54AM CDT, Andrew Jeffery wrote:
>Hi Zev,
>
>A couple of minor comments:
>
>On Thu, 8 Apr 2021, at 10:46, Zev Weiss wrote:
>> These allow describing all the Aspeed VUART attributes currently
>> available via sysfs. aspeed,sirq
>
>aspeed,lpc-interrupts now
Ah right, thanks.
>
>> provides a replacement for the
>> deprecated aspeed,sirq-polarity-sense property.
>>
>> Signed-off-by: Zev Weiss <[email protected]>
>> ---
>> drivers/tty/serial/8250/8250_aspeed_vuart.c | 44 ++++++++++++++++++++-
>> 1 file changed, 43 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c
>> b/drivers/tty/serial/8250/8250_aspeed_vuart.c
>> index 8433f8dbb186..75ef006fa24b 100644
>> --- a/drivers/tty/serial/8250/8250_aspeed_vuart.c
>> +++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c
>> @@ -28,6 +28,10 @@
>> #define ASPEED_VUART_ADDRL 0x28
>> #define ASPEED_VUART_ADDRH 0x2c
>>
>> +#define ASPEED_VUART_DEFAULT_LPC_ADDR 0x3f8
>> +#define ASPEED_VUART_DEFAULT_SIRQ 4
>> +#define ASPEED_VUART_DEFAULT_SIRQ_POLARITY IRQ_TYPE_LEVEL_LOW
>> +
>> struct aspeed_vuart {
>> struct device *dev;
>> void __iomem *regs;
>> @@ -393,7 +397,8 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
>> struct aspeed_vuart *vuart;
>> struct device_node *np;
>> struct resource *res;
>> - u32 clk, prop;
>> + u32 clk, prop, sirq[2];
>> + bool sirq_polarity;
>> int rc;
>>
>> np = pdev->dev.of_node;
>> @@ -501,6 +506,43 @@ static int aspeed_vuart_probe(struct platform_device *pdev)
>> of_node_put(sirq_polarity_sense_args.np);
>> }
>>
>> + rc = of_property_read_u32(np, "aspeed,lpc-io-reg", &prop);
>> + if (rc < 0)
>> + prop = ASPEED_VUART_DEFAULT_LPC_ADDR;
>> +
>> + rc = aspeed_vuart_set_lpc_address(vuart, prop);
>> + if (rc < 0) {
>> + dev_err(&pdev->dev, "invalid value in aspeed,lpc-io-reg property\n");
>> + goto err_clk_disable;
>> + }
>> +
>> + rc = of_property_read_u32_array(np, "aspeed,lpc-interrupts", sirq, 2);
>> + if (rc < 0) {
>> + sirq[0] = ASPEED_VUART_DEFAULT_SIRQ;
>> + sirq[1] = ASPEED_VUART_DEFAULT_SIRQ_POLARITY;
>> + }
>> +
>> + rc = aspeed_vuart_set_sirq(vuart, sirq[0]);
>> + if (rc < 0) {
>> + dev_err(&pdev->dev, "invalid sirq number in aspeed,lpc-interrupts > property\n");
>> + goto err_clk_disable;
>> + }
>> +
>> + switch (sirq[1]) {
>> + case IRQ_TYPE_LEVEL_LOW:
>> + sirq_polarity = false;
>> + break;
>> + case IRQ_TYPE_LEVEL_HIGH:
>> + sirq_polarity = true;
>> + break;
>> + default:
>> + dev_err(&pdev->dev, "invalid sirq polarity in aspeed,lpc-interrupts
>> property\n");
>> + rc = -EINVAL;
>> + goto err_clk_disable;
>> + }
>
>A bit ugly open-coding the mapping and error handling, maybe worth a helper?
>
Yeah, that sounds reasonable -- will do.
Thanks!
Zev