This patch set is example of the sort of driver changes needed to boot Juno using ACPI tables, which using the ACPI tables devloped for MS Windows and published by ARM [1].
For the smsc911x driver, it is based on the following ASL fragment which is part of DSDT for Juno:
// LAN9118 Ethernet // Device(ETH0) { Name(_HID, "ARMH9118") Name(_UID, Zero) Name(_CRS, ResourceTemplate() { Memory32Fixed(ReadWrite, 0x1A000000, 0x1000) Interrupt(ResourceConsumer, Level, ActiveHigh, Exclusive) { 192 } }) }
UART driver is just for review purpose, ARM is working on a more functional UART driver that does not poll to transmit. But it shows how to initialise SBSA compatible UART without clock definitions in DSDT.
You also can refer to the boot log from [2].
[1]: https://github.com/ARM-software/edk2/tree/juno-acpi/ArmPlatformPkg/ArmJunoPk... [2]: http://www.xora.org.uk/2014/08/29/juno-booting-from-acpi-tables/
Graeme Gregory (2): net: smsc911x add support for probing from ACPI tty: SBSA compatible UART
drivers/net/ethernet/smsc/smsc911x.c | 38 ++++ drivers/tty/Kconfig | 6 + drivers/tty/Makefile | 1 + drivers/tty/sbsauart.c | 328 ++++++++++++++++++++++++++++++++++ 4 files changed, 373 insertions(+) create mode 100644 drivers/tty/sbsauart.c
From: Graeme Gregory graeme.gregory@linaro.org
This is a standard platform device to resources are converted in the ACPI core in the same fasion as DT resources. For the other DT provided information there is _DSD for ACPI.
Signed-off-by: Graeme Gregory graeme.gregory@linaro.org --- drivers/net/ethernet/smsc/smsc911x.c | 38 ++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)
diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c index 5e13fa5..69d725a 100644 --- a/drivers/net/ethernet/smsc/smsc911x.c +++ b/drivers/net/ethernet/smsc/smsc911x.c @@ -59,6 +59,7 @@ #include <linux/of_device.h> #include <linux/of_gpio.h> #include <linux/of_net.h> +#include <linux/acpi.h> #include "smsc911x.h"
#define SMSC_CHIPNAME "smsc911x" @@ -2369,9 +2370,36 @@ static inline int smsc911x_probe_config_dt( } #endif /* CONFIG_OF */
+#ifdef CONFIG_ACPI +/* Configure some sensible defaults for ACPI mode */ +static int smsc911x_probe_config_acpi(struct smsc911x_platform_config *config, + acpi_handle *ahandle) +{ + if (!ahandle) + return -ENOSYS; + + config->phy_interface = PHY_INTERFACE_MODE_MII; + + config->flags |= SMSC911X_USE_32BIT; + + config->irq_polarity = SMSC911X_IRQ_POLARITY_ACTIVE_HIGH; + + config->irq_type = SMSC911X_IRQ_TYPE_PUSH_PULL; + + return 0; +} +#else +static int smsc911x_probe_config_acpi(struct smsc911x_platform_config *config, + acpi_handle *ahandle) +{ + return -ENOSYS; +} +#endif /* CONFIG_ACPI */ + static int smsc911x_drv_probe(struct platform_device *pdev) { struct device_node *np = pdev->dev.of_node; + acpi_handle *ahandle = ACPI_HANDLE(&pdev->dev); struct net_device *dev; struct smsc911x_data *pdata; struct smsc911x_platform_config *config = dev_get_platdata(&pdev->dev); @@ -2436,6 +2464,9 @@ static int smsc911x_drv_probe(struct platform_device *pdev) }
retval = smsc911x_probe_config_dt(&pdata->config, np); + if (retval) + retval = smsc911x_probe_config_acpi(&pdata->config, ahandle); + if (retval && config) { /* copy config parameters across to pdata */ memcpy(&pdata->config, config, sizeof(pdata->config)); @@ -2606,6 +2637,12 @@ static const struct of_device_id smsc911x_dt_ids[] = { MODULE_DEVICE_TABLE(of, smsc911x_dt_ids); #endif
+static const struct acpi_device_id smsc911x_acpi_ids[] = { + { "LNRO001B", }, + { "ARMH9118", }, + { } +}; + static struct platform_driver smsc911x_driver = { .probe = smsc911x_drv_probe, .remove = smsc911x_drv_remove, @@ -2614,6 +2651,7 @@ static struct platform_driver smsc911x_driver = { .owner = THIS_MODULE, .pm = SMSC911X_PM_OPS, .of_match_table = of_match_ptr(smsc911x_dt_ids), + .acpi_match_table = ACPI_PTR(smsc911x_acpi_ids), }, };
On Monday 01 September 2014 23:06:00 Hanjun Guo wrote:
+#ifdef CONFIG_ACPI +/* Configure some sensible defaults for ACPI mode */ +static int smsc911x_probe_config_acpi(struct smsc911x_platform_config *config,
acpi_handle *ahandle)+{
if (!ahandle)return -ENOSYS;config->phy_interface = PHY_INTERFACE_MODE_MII;
Please remove the #ifdef and use
if (!IS_ENABLED(CONFIG_ACPI) || !ahandle)
to check for ACPI support. This should result in the same object code in all cases, but give better compile-time coverage when ACPI is disabled.
Also, -ENOSYS is probably the wrong return value. I think you mean -ENXIO.
Arnd
On Mon, Sep 01, 2014 at 05:17:51PM +0200, Arnd Bergmann wrote:
On Monday 01 September 2014 23:06:00 Hanjun Guo wrote:
+#ifdef CONFIG_ACPI +/* Configure some sensible defaults for ACPI mode */ +static int smsc911x_probe_config_acpi(struct smsc911x_platform_config *config,
acpi_handle *ahandle)+{
if (!ahandle)return -ENOSYS;config->phy_interface = PHY_INTERFACE_MODE_MII;Please remove the #ifdef and use
if (!IS_ENABLED(CONFIG_ACPI) || !ahandle)
to check for ACPI support. This should result in the same object code in all cases, but give better compile-time coverage when ACPI is disabled.
struct acpi_handle does not exist in the case !CONFIG_ACPI
Also, -ENOSYS is probably the wrong return value. I think you mean -ENXIO.
Yes that would make sense thanks.
Not sure if we are planning to actually upstream this patch, I guess it depends if ARM start shipping Junos with the ACPI tables loaded on them.
Graeme
On Monday 01 September 2014 16:28:54 Graeme Gregory wrote:
On Mon, Sep 01, 2014 at 05:17:51PM +0200, Arnd Bergmann wrote:
On Monday 01 September 2014 23:06:00 Hanjun Guo wrote:
+#ifdef CONFIG_ACPI +/* Configure some sensible defaults for ACPI mode */ +static int smsc911x_probe_config_acpi(struct smsc911x_platform_config *config,
acpi_handle *ahandle)+{
if (!ahandle)return -ENOSYS;config->phy_interface = PHY_INTERFACE_MODE_MII;Please remove the #ifdef and use
if (!IS_ENABLED(CONFIG_ACPI) || !ahandle)to check for ACPI support. This should result in the same object code in all cases, but give better compile-time coverage when ACPI is disabled.
struct acpi_handle does not exist in the case !CONFIG_ACPI
That should be easy to fix, and a good idea in general, independent of this patch. We generally make function declarations and type definitions visible (possibly empty) for disabled subsystems so the code using them silently goes away when that subsystem is disabled.
Arnd
On Mon, Sep 01, 2014 at 04:28:54PM +0100, Graeme Gregory wrote:
On Mon, Sep 01, 2014 at 05:17:51PM +0200, Arnd Bergmann wrote:
On Monday 01 September 2014 23:06:00 Hanjun Guo wrote:
+#ifdef CONFIG_ACPI +/* Configure some sensible defaults for ACPI mode */ +static int smsc911x_probe_config_acpi(struct smsc911x_platform_config *config,
acpi_handle *ahandle)+{
if (!ahandle)return -ENOSYS;config->phy_interface = PHY_INTERFACE_MODE_MII;Please remove the #ifdef and use
if (!IS_ENABLED(CONFIG_ACPI) || !ahandle)
to check for ACPI support. This should result in the same object code in all cases, but give better compile-time coverage when ACPI is disabled.
struct acpi_handle does not exist in the case !CONFIG_ACPI
Confused. Then how come smsc911x_drv_probe() has this line:
acpi_handle *ahandle = ACPI_HANDLE(&pdev->dev);
without any #ifdef's.
On Mon, Sep 01, 2014 at 05:53:33PM +0100, Catalin Marinas wrote:
Confused. Then how come smsc911x_drv_probe() has this line:
acpi_handle *ahandle = ACPI_HANDLE(&pdev->dev);
without any #ifdef's.
There's a stub smsc911x_probe_config_acpi() provided in the non-ACPI case, Arnd's suggestion is basically to remove the stub.
Hi Mark,
On 01/09/14 17:58, Mark Brown wrote:
On Mon, Sep 01, 2014 at 05:53:33PM +0100, Catalin Marinas wrote:
Confused. Then how come smsc911x_drv_probe() has this line:
acpi_handle *ahandle = ACPI_HANDLE(&pdev->dev);
without any #ifdef's.
There's a stub smsc911x_probe_config_acpi() provided in the non-ACPI case, Arnd's suggestion is basically to remove the stub.
I think Catalin is referring to ACPI_HANDLE used without any #ifdefs
Catalin, few macros like ACPI_HANDLE and ACPI_PTR are defined in include/linux/acpi.h even when CONFIG_ACPI is not set mainly to avoid #ifdef's around simple assignments like the above one and one in platform_driver.acpi_match_table
Regards, Sudeep
On Mon, Sep 01, 2014 at 06:08:49PM +0100, Sudeep Holla wrote:
On 01/09/14 17:58, Mark Brown wrote:
On Mon, Sep 01, 2014 at 05:53:33PM +0100, Catalin Marinas wrote:
Confused. Then how come smsc911x_drv_probe() has this line:
acpi_handle *ahandle = ACPI_HANDLE(&pdev->dev);
without any #ifdef's.
There's a stub smsc911x_probe_config_acpi() provided in the non-ACPI case, Arnd's suggestion is basically to remove the stub.
I think Catalin is referring to ACPI_HANDLE used without any #ifdefs
Catalin, few macros like ACPI_HANDLE and ACPI_PTR are defined in include/linux/acpi.h even when CONFIG_ACPI is not set mainly to avoid #ifdef's around simple assignments like the above one and one in platform_driver.acpi_match_table
My comment was to Graeme who said that #ifdef's were needed because acpi_handle (lowercase) was not defined in the !CONFIG_ACPI case. However, further down in the patch it was used without any #ifdef's.
On 01/09/14 18:14, Catalin Marinas wrote:
On Mon, Sep 01, 2014 at 06:08:49PM +0100, Sudeep Holla wrote:
On 01/09/14 17:58, Mark Brown wrote:
On Mon, Sep 01, 2014 at 05:53:33PM +0100, Catalin Marinas wrote:
Confused. Then how come smsc911x_drv_probe() has this line:
acpi_handle *ahandle = ACPI_HANDLE(&pdev->dev);
without any #ifdef's.
There's a stub smsc911x_probe_config_acpi() provided in the non-ACPI case, Arnd's suggestion is basically to remove the stub.
I think Catalin is referring to ACPI_HANDLE used without any #ifdefs
Catalin, few macros like ACPI_HANDLE and ACPI_PTR are defined in include/linux/acpi.h even when CONFIG_ACPI is not set mainly to avoid #ifdef's around simple assignments like the above one and one in platform_driver.acpi_match_table
My comment was to Graeme who said that #ifdef's were needed because acpi_handle (lowercase) was not defined in the !CONFIG_ACPI case. However, further down in the patch it was used without any #ifdef's.
Ah OK, I misunderstood. In that case Graeme statement is wrong. IIRC acpi_handle is not structure, it's just a ptr used for object references in ACPI namespace and is available even when !CONFIG_ACPI
Regards, Sudeep
On Mon, Sep 01, 2014 at 05:53:33PM +0100, Catalin Marinas wrote:
On Mon, Sep 01, 2014 at 04:28:54PM +0100, Graeme Gregory wrote:
On Mon, Sep 01, 2014 at 05:17:51PM +0200, Arnd Bergmann wrote:
On Monday 01 September 2014 23:06:00 Hanjun Guo wrote:
+#ifdef CONFIG_ACPI +/* Configure some sensible defaults for ACPI mode */ +static int smsc911x_probe_config_acpi(struct smsc911x_platform_config *config,
acpi_handle *ahandle)+{
if (!ahandle)return -ENOSYS;config->phy_interface = PHY_INTERFACE_MODE_MII;Please remove the #ifdef and use
if (!IS_ENABLED(CONFIG_ACPI) || !ahandle)
to check for ACPI support. This should result in the same object code in all cases, but give better compile-time coverage when ACPI is disabled.
struct acpi_handle does not exist in the case !CONFIG_ACPI
Confused. Then how come smsc911x_drv_probe() has this line:
acpi_handle *ahandle = ACPI_HANDLE(&pdev->dev);
without any #ifdef's.
It is possible I confused myself as some types moved around in ACPI recently. I will re-check!
Graeme
On Mon, 1 Sep 2014 16:28:54 +0100, Graeme Gregory graeme.gregory@linaro.org wrote:
On Mon, Sep 01, 2014 at 05:17:51PM +0200, Arnd Bergmann wrote:
On Monday 01 September 2014 23:06:00 Hanjun Guo wrote:
+#ifdef CONFIG_ACPI +/* Configure some sensible defaults for ACPI mode */ +static int smsc911x_probe_config_acpi(struct smsc911x_platform_config *config,
acpi_handle *ahandle)+{
if (!ahandle)return -ENOSYS;config->phy_interface = PHY_INTERFACE_MODE_MII;Please remove the #ifdef and use
if (!IS_ENABLED(CONFIG_ACPI) || !ahandle)
to check for ACPI support. This should result in the same object code in all cases, but give better compile-time coverage when ACPI is disabled.
struct acpi_handle does not exist in the case !CONFIG_ACPI
Also, -ENOSYS is probably the wrong return value. I think you mean -ENXIO.
Yes that would make sense thanks.
Not sure if we are planning to actually upstream this patch, I guess it depends if ARM start shipping Junos with the ACPI tables loaded on them.
I think we do want this upstreamed. Juno is intended to be a readily available reference platform, regardless of the firmware loaded when shipped. There will be users who use it as a test platform for ACPI development.
g.
On Mon, Sep 15, 2014 at 05:08:44AM +0100, Grant Likely wrote:
On Mon, 1 Sep 2014 16:28:54 +0100, Graeme Gregory graeme.gregory@linaro.org wrote:
On Mon, Sep 01, 2014 at 05:17:51PM +0200, Arnd Bergmann wrote:
On Monday 01 September 2014 23:06:00 Hanjun Guo wrote:
+#ifdef CONFIG_ACPI +/* Configure some sensible defaults for ACPI mode */ +static int smsc911x_probe_config_acpi(struct smsc911x_platform_config *config,
acpi_handle *ahandle)+{
if (!ahandle)return -ENOSYS;config->phy_interface = PHY_INTERFACE_MODE_MII;Please remove the #ifdef and use
if (!IS_ENABLED(CONFIG_ACPI) || !ahandle)
to check for ACPI support. This should result in the same object code in all cases, but give better compile-time coverage when ACPI is disabled.
struct acpi_handle does not exist in the case !CONFIG_ACPI
Also, -ENOSYS is probably the wrong return value. I think you mean -ENXIO.
Yes that would make sense thanks.
Not sure if we are planning to actually upstream this patch, I guess it depends if ARM start shipping Junos with the ACPI tables loaded on them.
I think we do want this upstreamed. Juno is intended to be a readily available reference platform, regardless of the firmware loaded when shipped. There will be users who use it as a test platform for ACPI development.
The patches for Juno are not intrusive. The only problem I have is giving the wrong example on how to deal with the platform-specific device information like this patch. I would much prefer to use _DSD (once we agree on how to do this) than hard-coding information based on the ACPI device id (you would need one for each SoC). I don't think we have reached an agreement yet:
https://lkml.kernel.org/g/4816592.tj3on6vUaC@wuerfel
On Mon, Sep 01, 2014 at 04:06:00PM +0100, Hanjun Guo wrote:
+#ifdef CONFIG_ACPI +/* Configure some sensible defaults for ACPI mode */ +static int smsc911x_probe_config_acpi(struct smsc911x_platform_config *config,
acpi_handle *ahandle)+{
- if (!ahandle)
return -ENOSYS;- config->phy_interface = PHY_INTERFACE_MODE_MII;
- config->flags |= SMSC911X_USE_32BIT;
- config->irq_polarity = SMSC911X_IRQ_POLARITY_ACTIVE_HIGH;
- config->irq_type = SMSC911X_IRQ_TYPE_PUSH_PULL;
- return 0;
+} +#else
I don't like this and it shows issues we have with ACPI on certain ARM platforms. You hard-code these values to match the Juno platform. What if we get another SoC which has different configuration here? For DT, we have the smsc911x_probe_config_dt() which reads the relevant information from DT. I think this kind of configuration would be more suitable as _DSD properties and sharing the similar names with DT (but we go back to the question about who's in charge of the _DSD properties).
static int smsc911x_drv_probe(struct platform_device *pdev) { struct device_node *np = pdev->dev.of_node;
- acpi_handle *ahandle = ACPI_HANDLE(&pdev->dev); struct net_device *dev; struct smsc911x_data *pdata; struct smsc911x_platform_config *config = dev_get_platdata(&pdev->dev);
@@ -2436,6 +2464,9 @@ static int smsc911x_drv_probe(struct platform_device *pdev) } retval = smsc911x_probe_config_dt(&pdata->config, np);
- if (retval)
retval = smsc911x_probe_config_acpi(&pdata->config, ahandle);
In most of the ACPI patches so far we check for ACPI first with DT as a fall-back if ACPI is not enabled. This changes here. I would prefer something which probes only ACPI if the ACPI is enabled (run-time, not config) otherwise DT only. E.g.
On Monday 01 September 2014 18:04:47 Catalin Marinas wrote:
On Mon, Sep 01, 2014 at 04:06:00PM +0100, Hanjun Guo wrote:
+#ifdef CONFIG_ACPI +/* Configure some sensible defaults for ACPI mode */ +static int smsc911x_probe_config_acpi(struct smsc911x_platform_config *config,
acpi_handle *ahandle)+{
if (!ahandle)return -ENOSYS;config->phy_interface = PHY_INTERFACE_MODE_MII;config->flags |= SMSC911X_USE_32BIT;config->irq_polarity = SMSC911X_IRQ_POLARITY_ACTIVE_HIGH;config->irq_type = SMSC911X_IRQ_TYPE_PUSH_PULL;return 0;+} +#else
I don't like this and it shows issues we have with ACPI on certain ARM platforms. You hard-code these values to match the Juno platform. What if we get another SoC which has different configuration here? For DT, we have the smsc911x_probe_config_dt() which reads the relevant information from DT. I think this kind of configuration would be more suitable as _DSD properties and sharing the similar names with DT (but we go back to the question about who's in charge of the _DSD properties).
Good point, I totally missed that.
There is of course the possibility to set those values based on the acpi_device_id, but that is exactly the part that _DSD is trying to avoid.
static int smsc911x_drv_probe(struct platform_device *pdev) { struct device_node *np = pdev->dev.of_node;
acpi_handle *ahandle = ACPI_HANDLE(&pdev->dev); struct net_device *dev; struct smsc911x_data *pdata; struct smsc911x_platform_config *config = dev_get_platdata(&pdev->dev);@@ -2436,6 +2464,9 @@ static int smsc911x_drv_probe(struct platform_device *pdev) } retval = smsc911x_probe_config_dt(&pdata->config, np);
if (retval)retval = smsc911x_probe_config_acpi(&pdata->config, ahandle);In most of the ACPI patches so far we check for ACPI first with DT as a fall-back if ACPI is not enabled. This changes here.
Does this really make a difference?
I would prefer something which probes only ACPI if the ACPI is enabled (run-time, not config) otherwise DT only. E.g.
(example missing?)
I think we should have the equivalent of of_have_populated_dt(), to check whether acpi is being used to boot, and have that new function be hardcoded to zero in case of !IS_ENABLED(CONFIG_ACPI).
Arnd
On Mon, Sep 01, 2014 at 06:11:44PM +0100, Arnd Bergmann wrote:
On Monday 01 September 2014 18:04:47 Catalin Marinas wrote:
On Mon, Sep 01, 2014 at 04:06:00PM +0100, Hanjun Guo wrote:
+#ifdef CONFIG_ACPI +/* Configure some sensible defaults for ACPI mode */ +static int smsc911x_probe_config_acpi(struct smsc911x_platform_config *config,
acpi_handle *ahandle)+{
if (!ahandle)return -ENOSYS;config->phy_interface = PHY_INTERFACE_MODE_MII;config->flags |= SMSC911X_USE_32BIT;config->irq_polarity = SMSC911X_IRQ_POLARITY_ACTIVE_HIGH;config->irq_type = SMSC911X_IRQ_TYPE_PUSH_PULL;return 0;+} +#else
I don't like this and it shows issues we have with ACPI on certain ARM platforms. You hard-code these values to match the Juno platform. What if we get another SoC which has different configuration here? For DT, we have the smsc911x_probe_config_dt() which reads the relevant information from DT. I think this kind of configuration would be more suitable as _DSD properties and sharing the similar names with DT (but we go back to the question about who's in charge of the _DSD properties).
Good point, I totally missed that.
There is of course the possibility to set those values based on the acpi_device_id, but that is exactly the part that _DSD is trying to avoid.
I would prefer to avoid acpi_device_id checks. This would defeat the alleged aim of ACPI to run newer hardware configuration with older kernels.
static int smsc911x_drv_probe(struct platform_device *pdev) { struct device_node *np = pdev->dev.of_node;
acpi_handle *ahandle = ACPI_HANDLE(&pdev->dev); struct net_device *dev; struct smsc911x_data *pdata; struct smsc911x_platform_config *config = dev_get_platdata(&pdev->dev);@@ -2436,6 +2464,9 @@ static int smsc911x_drv_probe(struct platform_device *pdev) } retval = smsc911x_probe_config_dt(&pdata->config, np);
if (retval)retval = smsc911x_probe_config_acpi(&pdata->config, ahandle);In most of the ACPI patches so far we check for ACPI first with DT as a fall-back if ACPI is not enabled. This changes here.
Does this really make a difference?
Not the order (well, someone may think that if they unflatten the DT in a vendor kernel even though it boot as ACPI, they could get away with a mix of DT and ACPI ;)).
I would prefer something which probes only ACPI if the ACPI is enabled (run-time, not config) otherwise DT only. E.g.
(example missing?)
I was looking through the code and forgot. Something like:
if (acpi_disabled) retval = smsc911x_probe_config_dt(); else retval = smsc911x_probe_config_acpi();
I think we should have the equivalent of of_have_populated_dt(), to check whether acpi is being used to boot, and have that new function be hardcoded to zero in case of !IS_ENABLED(CONFIG_ACPI).
I think you meant hardcoded to 1 when !ACPI. That would work as well.
On Mon, Sep 01, 2014 at 07:11:44PM +0200, Arnd Bergmann wrote:
On Monday 01 September 2014 18:04:47 Catalin Marinas wrote:
On Mon, Sep 01, 2014 at 04:06:00PM +0100, Hanjun Guo wrote:
+#ifdef CONFIG_ACPI +/* Configure some sensible defaults for ACPI mode */ +static int smsc911x_probe_config_acpi(struct smsc911x_platform_config *config,
acpi_handle *ahandle)+{
if (!ahandle)return -ENOSYS;config->phy_interface = PHY_INTERFACE_MODE_MII;config->flags |= SMSC911X_USE_32BIT;config->irq_polarity = SMSC911X_IRQ_POLARITY_ACTIVE_HIGH;config->irq_type = SMSC911X_IRQ_TYPE_PUSH_PULL;return 0;+} +#else
I don't like this and it shows issues we have with ACPI on certain ARM platforms. You hard-code these values to match the Juno platform. What if we get another SoC which has different configuration here? For DT, we have the smsc911x_probe_config_dt() which reads the relevant information from DT. I think this kind of configuration would be more suitable as _DSD properties and sharing the similar names with DT (but we go back to the question about who's in charge of the _DSD properties).
Good point, I totally missed that.
There is of course the possibility to set those values based on the acpi_device_id, but that is exactly the part that _DSD is trying to avoid.
This will of course most likely be replaced by _DSD values. I just hardcoded for now as _DSD is not yet in the kernel and issues around maintenance of bindings are not solved (unless this happened at KS where I was not present).
static int smsc911x_drv_probe(struct platform_device *pdev) { struct device_node *np = pdev->dev.of_node;
acpi_handle *ahandle = ACPI_HANDLE(&pdev->dev); struct net_device *dev; struct smsc911x_data *pdata; struct smsc911x_platform_config *config = dev_get_platdata(&pdev->dev);@@ -2436,6 +2464,9 @@ static int smsc911x_drv_probe(struct platform_device *pdev) } retval = smsc911x_probe_config_dt(&pdata->config, np);
if (retval)retval = smsc911x_probe_config_acpi(&pdata->config, ahandle);In most of the ACPI patches so far we check for ACPI first with DT as a fall-back if ACPI is not enabled. This changes here.
Does this really make a difference?
I would prefer something which probes only ACPI if the ACPI is enabled (run-time, not config) otherwise DT only. E.g.
(example missing?)
I think we should have the equivalent of of_have_populated_dt(), to check whether acpi is being used to boot, and have that new function be hardcoded to zero in case of !IS_ENABLED(CONFIG_ACPI).
if (!acpi_disabled) is the equivalent if I understand you correctly.
But people until this point had expressed a preference for checking .of_node and ACPI_HANDLE() to this point. This is obviously mutable though depending on community preference.
Graeme
On Mon, Sep 01, 2014 at 06:32:45PM +0100, Graeme Gregory wrote:
This will of course most likely be replaced by _DSD values. I just hardcoded for now as _DSD is not yet in the kernel and issues around maintenance of bindings are not solved (unless this happened at KS where I was not present).
Not really. AIUI the official thing is still some combination of getting a DT binding defined and then use that verbatim (as the x86 embedded people want to though I'm still not clear that the Windows people are on board with this) and the UEFI processes. It's possible I misunderstood though.
Realistically if something is usable on x86 embedded it's going to be supported on ARMv8 servers too unless we jump through hoops, and even there real implementations in the wild will probably win out.
On Mon, Sep 01, 2014 at 06:32:45PM +0100, Graeme Gregory wrote:
On Mon, Sep 01, 2014 at 07:11:44PM +0200, Arnd Bergmann wrote:
On Monday 01 September 2014 18:04:47 Catalin Marinas wrote:
On Mon, Sep 01, 2014 at 04:06:00PM +0100, Hanjun Guo wrote:
+#ifdef CONFIG_ACPI +/* Configure some sensible defaults for ACPI mode */ +static int smsc911x_probe_config_acpi(struct smsc911x_platform_config *config,
acpi_handle *ahandle)+{
if (!ahandle)return -ENOSYS;config->phy_interface = PHY_INTERFACE_MODE_MII;config->flags |= SMSC911X_USE_32BIT;config->irq_polarity = SMSC911X_IRQ_POLARITY_ACTIVE_HIGH;config->irq_type = SMSC911X_IRQ_TYPE_PUSH_PULL;return 0;+} +#else
I don't like this and it shows issues we have with ACPI on certain ARM platforms. You hard-code these values to match the Juno platform. What if we get another SoC which has different configuration here? For DT, we have the smsc911x_probe_config_dt() which reads the relevant information from DT. I think this kind of configuration would be more suitable as _DSD properties and sharing the similar names with DT (but we go back to the question about who's in charge of the _DSD properties).
Good point, I totally missed that.
There is of course the possibility to set those values based on the acpi_device_id, but that is exactly the part that _DSD is trying to avoid.
This will of course most likely be replaced by _DSD values. I just hardcoded for now as _DSD is not yet in the kernel and issues around maintenance of bindings are not solved (unless this happened at KS where I was not present).
Not much at the KS, I think it will need to be followed up on lkml (https://lkml.org/lkml/2014/8/17/10 is the last I'm aware of, not sure about any updates in the meantime).
While the above gets sorted, what's the position from an ARM perspective (and covered by Documentation/arm64/arm-acpi.txt)? I think the "Device Enumeration" section in this document is fine, it's just the kernel infrastructure missing.
Alternatively, you can say _DSD is not allowed (yet?) but I don't particularly like basing the configuration on acpi_device_id like in this patch. Which would leave us with ignoring any SoC containing devices that require such specific configuration.
On Tuesday 02 September 2014 14:26:52 Catalin Marinas wrote:
Not much at the KS, I think it will need to be followed up on lkml (https://lkml.org/lkml/2014/8/17/10 is the last I'm aware of, not sure about any updates in the meantime).
While the above gets sorted, what's the position from an ARM perspective (and covered by Documentation/arm64/arm-acpi.txt)? I think the "Device Enumeration" section in this document is fine, it's just the kernel infrastructure missing.
Alternatively, you can say _DSD is not allowed (yet?) but I don't particularly like basing the configuration on acpi_device_id like in this patch. Which would leave us with ignoring any SoC containing devices that require such specific configuration.
The way I recall the discussion, most people were on one extreme side of the discussion or the other:
a) We should use _DSD for ARM64 servers to maximize code reuse with DT-enabled drivers, work around the slow UEFI standardization process, remain in control of the actual bindings, and avoid the need for endless per-ID platform-data definitions in drivers.
b) We should never use _DSD at all, since doing that would have no advantage over using DT directly, and we should force every device manufacturer to specify their bindings in an official ACPI document to prevent random incompatible bindings from being established. Any device that shows up in servers should not need arbitrary detailed properties anyway, as the details are supposed to be hidden in AML.
I can understand the reasons for both approaches, and I find it hard to say either one is invalid. However, the worst possible outcome in my opinion would be having to support a mix of the two.
Arnd
On Tue, Sep 02, 2014 at 03:42:53PM +0200, Arnd Bergmann wrote:
The way I recall the discussion, most people were on one extreme side of the discussion or the other:
a) We should use _DSD for ARM64 servers to maximize code reuse with DT-enabled drivers, work around the slow UEFI standardization process, remain in control of the actual bindings, and avoid the need for endless per-ID platform-data definitions in drivers.
b) We should never use _DSD at all, since doing that would have no advantage over using DT directly, and we should force every device manufacturer to specify their bindings in an official ACPI document to prevent random incompatible bindings from being established. Any device that shows up in servers should not need arbitrary detailed properties anyway, as the details are supposed to be hidden in AML.
I can understand the reasons for both approaches, and I find it hard to say either one is invalid. However, the worst possible outcome in my opinion would be having to support a mix of the two.
Right, and the x86 embedded folks are going full steam ahead with _DSD regardless so it seems there will be some systems out there using it even if they're not ARM servers.
On Tuesday, September 02, 2014 05:26:06 PM Mark Brown wrote:
--s3puAW9DMBtS2ARW Content-Type: text/plain; charset=us-ascii Content-Disposition: inline
On Tue, Sep 02, 2014 at 03:42:53PM +0200, Arnd Bergmann wrote:
The way I recall the discussion, most people were on one extreme side of the discussion or the other:
a) We should use _DSD for ARM64 servers to maximize code reuse with DT-enabled drivers, work around the slow UEFI standardization process, remain in control of the actual bindings, and avoid the need for endless per-ID platform-data definitions in drivers.
b) We should never use _DSD at all, since doing that would have no advantage over using DT directly, and we should force every device manufacturer to specify their bindings in an official ACPI document to prevent random incompatible bindings from being established. Any device that shows up in servers should not need arbitrary detailed properties anyway, as the details are supposed to be hidden in AML.
I can understand the reasons for both approaches, and I find it hard to say either one is invalid. However, the worst possible outcome in my opinion would be having to support a mix of the two.
Right, and the x86 embedded folks are going full steam ahead with _DSD regardless so it seems there will be some systems out there using it even if they're not ARM servers.
Our intention is specifically not to use "random incompatible bindings" in that. We'd rather have a common venue and process for establishing new bindings for both DT and _DSD in a compatible way.
Rafael
On Wednesday 03 September 2014 01:00:23 Rafael J. Wysocki wrote:
On Tuesday, September 02, 2014 05:26:06 PM Mark Brown wrote:
--s3puAW9DMBtS2ARW Content-Type: text/plain; charset=us-ascii Content-Disposition: inline
On Tue, Sep 02, 2014 at 03:42:53PM +0200, Arnd Bergmann wrote:
The way I recall the discussion, most people were on one extreme side of the discussion or the other:
a) We should use _DSD for ARM64 servers to maximize code reuse with DT-enabled drivers, work around the slow UEFI standardization process, remain in control of the actual bindings, and avoid the need for endless per-ID platform-data definitions in drivers.
b) We should never use _DSD at all, since doing that would have no advantage over using DT directly, and we should force every device manufacturer to specify their bindings in an official ACPI document to prevent random incompatible bindings from being established. Any device that shows up in servers should not need arbitrary detailed properties anyway, as the details are supposed to be hidden in AML.
I can understand the reasons for both approaches, and I find it hard to say either one is invalid. However, the worst possible outcome in my opinion would be having to support a mix of the two.
Right, and the x86 embedded folks are going full steam ahead with _DSD regardless so it seems there will be some systems out there using it even if they're not ARM servers.
Our intention is specifically not to use "random incompatible bindings" in that. We'd rather have a common venue and process for establishing new bindings for both DT and _DSD in a compatible way.
Right, I think everyone is on the same page for the embedded x86 case, my point was that there is no consensus about that yet among the parties involved in arm64 servers.
Arnd
Hi Arnd,
On 09/03/2014 11:09 AM, Arnd Bergmann wrote:
On Wednesday 03 September 2014 01:00:23 Rafael J. Wysocki wrote:
Our intention is specifically not to use "random incompatible bindings" in that. We'd rather have a common venue and process for establishing new bindings for both DT and _DSD in a compatible way.
Right, I think everyone is on the same page for the embedded x86 case, my point was that there is no consensus about that yet among the parties involved in arm64 servers.
However, in the case of MAC devices where you might need to specify just a couple of properties using _DSD, a specific set of discussions has been instigated. As you mentioned, though, this pertains only to certain devices and isn't necessarily true for every Ethernet device. Consequently, while a Juno PoC patch might use a kludge today, that doesn't mean it's a limitation of ACPI, just of certain devices.
Jon.
On Mon, 01 Sep 2014 19:11:44 +0200, Arnd Bergmann arnd@arndb.de wrote:
On Monday 01 September 2014 18:04:47 Catalin Marinas wrote:
On Mon, Sep 01, 2014 at 04:06:00PM +0100, Hanjun Guo wrote:
+#ifdef CONFIG_ACPI +/* Configure some sensible defaults for ACPI mode */ +static int smsc911x_probe_config_acpi(struct smsc911x_platform_config *config,
acpi_handle *ahandle)+{
if (!ahandle)return -ENOSYS;config->phy_interface = PHY_INTERFACE_MODE_MII;config->flags |= SMSC911X_USE_32BIT;config->irq_polarity = SMSC911X_IRQ_POLARITY_ACTIVE_HIGH;config->irq_type = SMSC911X_IRQ_TYPE_PUSH_PULL;return 0;+} +#else
I don't like this and it shows issues we have with ACPI on certain ARM platforms. You hard-code these values to match the Juno platform. What if we get another SoC which has different configuration here? For DT, we have the smsc911x_probe_config_dt() which reads the relevant information from DT. I think this kind of configuration would be more suitable as _DSD properties and sharing the similar names with DT (but we go back to the question about who's in charge of the _DSD properties).
Good point, I totally missed that.
There is of course the possibility to set those values based on the acpi_device_id, but that is exactly the part that _DSD is trying to avoid.
These are merely defaults. DSD parsing, when implemented, would be override these default values.
static int smsc911x_drv_probe(struct platform_device *pdev) { struct device_node *np = pdev->dev.of_node;
acpi_handle *ahandle = ACPI_HANDLE(&pdev->dev); struct net_device *dev; struct smsc911x_data *pdata; struct smsc911x_platform_config *config = dev_get_platdata(&pdev->dev);@@ -2436,6 +2464,9 @@ static int smsc911x_drv_probe(struct platform_device *pdev) } retval = smsc911x_probe_config_dt(&pdata->config, np);
if (retval)retval = smsc911x_probe_config_acpi(&pdata->config, ahandle);In most of the ACPI patches so far we check for ACPI first with DT as a fall-back if ACPI is not enabled. This changes here.
Does this really make a difference?
Nope. Only one of DT or ACPI will be matched.
I would prefer something which probes only ACPI if the ACPI is enabled (run-time, not config) otherwise DT only. E.g.
(example missing?)
I think we should have the equivalent of of_have_populated_dt(), to check whether acpi is being used to boot, and have that new function be hardcoded to zero in case of !IS_ENABLED(CONFIG_ACPI).
The code already accounts for it. If ACPI isn't enabled, or isn't populated, then the ACPI_HANDLE macro will return NULL and the smsc911x_probe_config_acpi() function will fail.
g.
On Sun, Sep 14, 2014 at 09:14:04PM -0700, Grant Likely wrote:
On Mon, 01 Sep 2014 19:11:44 +0200, Arnd Bergmann arnd@arndb.de wrote:
config->phy_interface = PHY_INTERFACE_MODE_MII;
config->flags |= SMSC911X_USE_32BIT;
config->irq_polarity = SMSC911X_IRQ_POLARITY_ACTIVE_HIGH;
config->irq_type = SMSC911X_IRQ_TYPE_PUSH_PULL;
return 0;+}
...
There is of course the possibility to set those values based on the acpi_device_id, but that is exactly the part that _DSD is trying to avoid.
These are merely defaults. DSD parsing, when implemented, would be override these default values.
One note of caution here: I do agree that default settings are good but it's worth having clear rules for how we pick the defaults, and advice for maintainers on how to pick those rules. If there's a default people often want to pick it to match their particular system and it can sometimes be hard for the maintainer to identify if a given tweak someone is proposing in the defaults might break some other existing system. Having clear guidelines for picking the defaults avoids arguments and breakage.
The two basic rules I've seen are that we either follow the defaults the hardware has after reset or we follow the state the hardware is left in when the kernel starts. Neither is perfect and sometimes the bootloader option just doesn't make sense at all but they're at least clear and simple to understand.
It's not the end of the world to do something else, and sometimes the way systems are done just doesn't lend itself to providing clear rules, but if we encourage people to set them they can save grief.
From: Graeme Gregory graeme.gregory@linaro.org
This is a subset of pl011 UART which does not supprt DMA or baud rate changing.
It is specified in the Server Base System Architecture document from ARM.
Signed-off-by: Graeme Gregory graeme.gregory@linaro.org --- drivers/tty/Kconfig | 6 + drivers/tty/Makefile | 1 + drivers/tty/sbsauart.c | 328 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 335 insertions(+) create mode 100644 drivers/tty/sbsauart.c
diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig index b24aa01..50fe279 100644 --- a/drivers/tty/Kconfig +++ b/drivers/tty/Kconfig @@ -419,4 +419,10 @@ config DA_CONSOLE help This enables a console on a Dash channel.
+config SBSAUART_TTY + tristate "SBSA UART TTY Driver" + help + Console and system TTY driver for the SBSA UART which is defined + in the Server Base System Architecure document for ARM64 servers. + endif # TTY diff --git a/drivers/tty/Makefile b/drivers/tty/Makefile index 58ad1c0..c3211c0 100644 --- a/drivers/tty/Makefile +++ b/drivers/tty/Makefile @@ -29,5 +29,6 @@ obj-$(CONFIG_SYNCLINK) += synclink.o obj-$(CONFIG_PPC_EPAPR_HV_BYTECHAN) += ehv_bytechan.o obj-$(CONFIG_GOLDFISH_TTY) += goldfish.o obj-$(CONFIG_DA_TTY) += metag_da.o +obj-$(CONFIG_SBSAUART_TTY) += sbsauart.o
obj-y += ipwireless/ diff --git a/drivers/tty/sbsauart.c b/drivers/tty/sbsauart.c new file mode 100644 index 0000000..3a3ff88 --- /dev/null +++ b/drivers/tty/sbsauart.c @@ -0,0 +1,328 @@ +/* + * SBSA (Server Base System Architecture) Compatible UART driver + * + * Copyright (C) 2014 Linaro Ltd + * + * Author: Graeme Gregory graeme.gregory@linaro.org + * + * This software is licensed under the terms of the GNU General Public + * License version 2, as published by the Free Software Foundation, and + * may be copied, distributed, and modified under those terms. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +#include <linux/acpi.h> +#include <linux/amba/serial.h> +#include <linux/console.h> +#include <linux/delay.h> +#include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/slab.h> +#include <linux/tty.h> +#include <linux/tty_flip.h> + +struct sbsa_tty { + struct tty_port port; + spinlock_t lock; + void __iomem *base; + u32 irq; + int opencount; + struct console console; +}; + +static struct tty_driver *sbsa_tty_driver; +static struct sbsa_tty *sbsa_tty; + +#define SBSAUART_CHAR_MASK 0xFF + +static void sbsa_tty_do_write(const char *buf, unsigned count) +{ + unsigned long irq_flags; + struct sbsa_tty *qtty = sbsa_tty; + void __iomem *base = qtty->base; + unsigned n; + + spin_lock_irqsave(&qtty->lock, irq_flags); + for (n = 0; n < count; n++) { + while (readw(base + UART01x_FR) & UART01x_FR_TXFF) + mdelay(10); + writew(buf[n], base + UART01x_DR); + } + spin_unlock_irqrestore(&qtty->lock, irq_flags); +} + +static void sbsauart_fifo_to_tty(struct sbsa_tty *qtty) +{ + void __iomem *base = qtty->base; + unsigned int flag, max_count = 32; + u16 status, ch; + + while (max_count--) { + status = readw(base + UART01x_FR); + if (status & UART01x_FR_RXFE) + break; + + /* Take chars from the FIFO and update status */ + ch = readw(base + UART01x_DR); + flag = TTY_NORMAL; + + if (ch & UART011_DR_BE) + flag = TTY_BREAK; + else if (ch & UART011_DR_PE) + flag = TTY_PARITY; + else if (ch & UART011_DR_FE) + flag = TTY_FRAME; + else if (ch & UART011_DR_OE) + flag = TTY_OVERRUN; + + ch &= SBSAUART_CHAR_MASK; + + tty_insert_flip_char(&qtty->port, ch, flag); + } + + tty_schedule_flip(&qtty->port); + + /* Clear the RX IRQ */ + writew(UART011_RXIC | UART011_RXIC, base + UART011_ICR); +} + +static irqreturn_t sbsa_tty_interrupt(int irq, void *dev_id) +{ + struct sbsa_tty *qtty = sbsa_tty; + unsigned long irq_flags; + + spin_lock_irqsave(&qtty->lock, irq_flags); + sbsauart_fifo_to_tty(qtty); + spin_unlock_irqrestore(&qtty->lock, irq_flags); + + return IRQ_HANDLED; +} + +static int sbsa_tty_open(struct tty_struct *tty, struct file *filp) +{ + struct sbsa_tty *qtty = sbsa_tty; + + return tty_port_open(&qtty->port, tty, filp); +} + +static void sbsa_tty_close(struct tty_struct *tty, struct file *filp) +{ + tty_port_close(tty->port, tty, filp); +} + +static void sbsa_tty_hangup(struct tty_struct *tty) +{ + tty_port_hangup(tty->port); +} + +static int sbsa_tty_write(struct tty_struct *tty, const unsigned char *buf, + int count) +{ + sbsa_tty_do_write(buf, count); + return count; +} + +static int sbsa_tty_write_room(struct tty_struct *tty) +{ + return 32; +} + +static void sbsa_tty_console_write(struct console *co, const char *b, + unsigned count) +{ + sbsa_tty_do_write(b, count); + + if(b[count - 1] == '\n'); + sbsa_tty_do_write("\r", 1); +} + +static struct tty_driver *sbsa_tty_console_device(struct console *c, + int *index) +{ + *index = c->index; + return sbsa_tty_driver; +} + +static int sbsa_tty_console_setup(struct console *co, char *options) +{ + if ((unsigned)co->index > 0) + return -ENODEV; + if (sbsa_tty->base == NULL) + return -ENODEV; + return 0; +} + +static struct tty_port_operations sbsa_port_ops = { +}; + +static const struct tty_operations sbsa_tty_ops = { + .open = sbsa_tty_open, + .close = sbsa_tty_close, + .hangup = sbsa_tty_hangup, + .write = sbsa_tty_write, + .write_room = sbsa_tty_write_room, +}; + +static int sbsa_tty_create_driver(void) +{ + int ret; + struct tty_driver *tty; + + sbsa_tty = kzalloc(sizeof(*sbsa_tty), GFP_KERNEL); + if (sbsa_tty == NULL) { + ret = -ENOMEM; + goto err_alloc_sbsa_tty_failed; + } + tty = alloc_tty_driver(1); + if (tty == NULL) { + ret = -ENOMEM; + goto err_alloc_tty_driver_failed; + } + tty->driver_name = "sbsauart"; + tty->name = "ttySBSA"; + tty->type = TTY_DRIVER_TYPE_SERIAL; + tty->subtype = SERIAL_TYPE_NORMAL; + tty->init_termios = tty_std_termios; + tty->flags = TTY_DRIVER_RESET_TERMIOS | TTY_DRIVER_REAL_RAW | + TTY_DRIVER_DYNAMIC_DEV; + tty_set_operations(tty, &sbsa_tty_ops); + ret = tty_register_driver(tty); + if (ret) + goto err_tty_register_driver_failed; + + sbsa_tty_driver = tty; + return 0; + +err_tty_register_driver_failed: + put_tty_driver(tty); +err_alloc_tty_driver_failed: + kfree(sbsa_tty); + sbsa_tty = NULL; +err_alloc_sbsa_tty_failed: + return ret; +} + +static void sbsa_tty_delete_driver(void) +{ + tty_unregister_driver(sbsa_tty_driver); + put_tty_driver(sbsa_tty_driver); + sbsa_tty_driver = NULL; + kfree(sbsa_tty); + sbsa_tty = NULL; +} + +static int sbsa_tty_probe(struct platform_device *pdev) +{ + struct sbsa_tty *qtty; + int ret = -EINVAL; + int i; + struct resource *r; + struct device *ttydev; + void __iomem *base; + u32 irq; + + r = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (r == NULL) + return -EINVAL; + + base = ioremap(r->start, r->end - r->start); + if (base == NULL) + pr_err("sbsa_tty: unable to remap base\n"); + + r = platform_get_resource(pdev, IORESOURCE_IRQ, 0); + if (r == NULL) + goto err_unmap; + + irq = r->start; + + if (pdev->id > 0) + goto err_unmap; + + ret = sbsa_tty_create_driver(); + if (ret) + goto err_unmap; + + qtty = sbsa_tty; + spin_lock_init(&qtty->lock); + tty_port_init(&qtty->port); + qtty->port.ops = &sbsa_port_ops; + qtty->base = base; + qtty->irq = irq; + + /* Clear and Mask all IRQs */ + writew(0, base + UART011_IMSC); + writew(0xFFFF, base + UART011_ICR); + + ret = request_irq(irq, sbsa_tty_interrupt, IRQF_SHARED, + "sbsa_tty", pdev); + if (ret) + goto err_request_irq_failed; + + /* Unmask the RX IRQ */ + writew(UART011_RXIM | UART011_RTIM, base + UART011_IMSC); + + ttydev = tty_port_register_device(&qtty->port, sbsa_tty_driver, + 0, &pdev->dev); + if (IS_ERR(ttydev)) { + ret = PTR_ERR(ttydev); + goto err_tty_register_device_failed; + } + + strcpy(qtty->console.name, "ttySBSA"); + qtty->console.write = sbsa_tty_console_write; + qtty->console.device = sbsa_tty_console_device; + qtty->console.setup = sbsa_tty_console_setup; + qtty->console.flags = CON_PRINTBUFFER; + qtty->console.index = pdev->id; + register_console(&qtty->console); + + return 0; + + tty_unregister_device(sbsa_tty_driver, i); +err_tty_register_device_failed: + free_irq(irq, pdev); +err_request_irq_failed: + sbsa_tty_delete_driver(); +err_unmap: + iounmap(base); + return ret; +} + +static int sbsa_tty_remove(struct platform_device *pdev) +{ + struct sbsa_tty *qtty; + + qtty = sbsa_tty; + unregister_console(&qtty->console); + tty_unregister_device(sbsa_tty_driver, pdev->id); + iounmap(qtty->base); + qtty->base = 0; + free_irq(qtty->irq, pdev); + sbsa_tty_delete_driver(); + return 0; +} + +static const struct acpi_device_id sbsa_acpi_match[] = { + { "ARMH0011", 0 }, + { } +}; + +static struct platform_driver sbsa_tty_platform_driver = { + .probe = sbsa_tty_probe, + .remove = sbsa_tty_remove, + .driver = { + .name = "sbsa_tty", + .acpi_match_table = ACPI_PTR(sbsa_acpi_match), + } +}; + +module_platform_driver(sbsa_tty_platform_driver); + +MODULE_LICENSE("GPL v2");
On Mon, Sep 01, 2014 at 11:06:01PM +0800, Hanjun Guo wrote:
From: Graeme Gregory graeme.gregory@linaro.org
This is a subset of pl011 UART which does not supprt DMA or baud rate changing.
It is specified in the Server Base System Architecture document from ARM.
Is there any reason not to consider submitting this as a TTY driver now - are there known problems other than the issues with ACPI in general (I've not reviewed the code at all)? The ACPI binding in this case seems particularly safe since it's a purposely basic and unconfigurable IP.
I guess we may want to consider handoff to the full pl011 driver at some point but there's a bunch of stuff with clocks to worry about before then.
Signed-off-by: Graeme Gregory graeme.gregory@linaro.org
Missing signoff here.
+config SBSAUART_TTY
- tristate "SBSA UART TTY Driver"
- help
Console and system TTY driver for the SBSA UART which is definedin the Server Base System Architecure document for ARM64 servers.
depends on ARM || ARM64 || COMPILE_TEST?
On Mon, Sep 01, 2014 at 04:06:01PM +0100, Hanjun Guo wrote:
From: Graeme Gregory graeme.gregory@linaro.org
This is a subset of pl011 UART which does not supprt DMA or baud rate changing.
It is specified in the Server Base System Architecture document from ARM.
Signed-off-by: Graeme Gregory graeme.gregory@linaro.org
drivers/tty/Kconfig | 6 + drivers/tty/Makefile | 1 + drivers/tty/sbsauart.c | 328 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 335 insertions(+) create mode 100644 drivers/tty/sbsauart.c
Any overlap with this?
http://www.spinics.net/lists/arm-kernel/msg358084.html
On Mon, Sep 01, 2014 at 06:12:48PM +0100, Catalin Marinas wrote:
On Mon, Sep 01, 2014 at 04:06:01PM +0100, Hanjun Guo wrote:
From: Graeme Gregory graeme.gregory@linaro.org
This is a subset of pl011 UART which does not supprt DMA or baud rate changing.
It is specified in the Server Base System Architecture document from ARM.
Signed-off-by: Graeme Gregory graeme.gregory@linaro.org
drivers/tty/Kconfig | 6 + drivers/tty/Makefile | 1 + drivers/tty/sbsauart.c | 328 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 335 insertions(+) create mode 100644 drivers/tty/sbsauart.c
Any overlap with this?
I knew that driver was being developed and is why we specifically said we did not plan to upstream this in preference for ARMs version.
I just missed the release of this 2 days ago.
Graeme
On Mon, Sep 01, 2014 at 06:18:56PM +0100, Graeme Gregory wrote:
On Mon, Sep 01, 2014 at 06:12:48PM +0100, Catalin Marinas wrote:
On Mon, Sep 01, 2014 at 04:06:01PM +0100, Hanjun Guo wrote:
From: Graeme Gregory graeme.gregory@linaro.org
This is a subset of pl011 UART which does not supprt DMA or baud rate changing.
It is specified in the Server Base System Architecture document from ARM.
Signed-off-by: Graeme Gregory graeme.gregory@linaro.org
drivers/tty/Kconfig | 6 + drivers/tty/Makefile | 1 + drivers/tty/sbsauart.c | 328 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 335 insertions(+) create mode 100644 drivers/tty/sbsauart.c
Any overlap with this?
I knew that driver was being developed and is why we specifically said we did not plan to upstream this in preference for ARMs version.
I just missed the release of this 2 days ago.
Thanks. Please give feedback to Andre when you had the chance to look at it.
On Monday 01 September 2014 23:05:59 Hanjun Guo wrote:
This patch set is example of the sort of driver changes needed to boot Juno using ACPI tables, which using the ACPI tables devloped for MS Windows and published by ARM [1].
What about platform support?
I see that there is a patch in your git tree at https://git.linaro.org/leg/acpi/acpi.git/commitdiff/b9a635e3b4cb167d399 that exposes the raw system registers in the same way that the DT support does, which I guess is not how you plan to do it for ACPI when you are done.
Is that still work in progress? How do you plan to abstract things like the hwmon, LEDs, GPIO etc that are handled through the sysregs in hardware?
Arnd
-
Arnd Bergmann -
Catalin Marinas -
Graeme Gregory -
Graeme Gregory -
Grant Likely -
Grant Likely -
Hanjun Guo -
Jon Masters -
Mark Brown -
Rafael J. Wysocki -
Sudeep Holla