* [PATCH 0/5] platform/x86: x86-android-tablets: Add Bluetooth support for Vexia EDU ATLA 10
@ 2024-11-09 22:05 Hans de Goede
2024-11-09 22:05 ` [PATCH 1/5] platform/x86: serdev_helpers: Add get_serdev_controller_from_parent() helper Hans de Goede
` (5 more replies)
0 siblings, 6 replies; 11+ messages in thread
From: Hans de Goede @ 2024-11-09 22:05 UTC (permalink / raw)
To: Ilpo Järvinen, Andy Shevchenko; +Cc: Hans de Goede, platform-driver-x86
Hi All,
Here is a patch-series for adding Bluetooth support for the Vexia EDU ATLA
10 tablet to x86-android-tablets.
Due to the LPSS UARTs being enumerated through PCI rather then through
ACPI, this is somewhat involved. Just like how this special case needed
some extra work for instantiating the various i2c-clients.
Regards,
Hans
Hans de Goede (5):
platform/x86: serdev_helpers: Add get_serdev_controller_from_parent()
helper
platform/x86: x86-android-tablets: Add missing __init to
get_i2c_adap_by_*()
platform/x86: x86-android-tablets: Change x86_instantiate_serdev()
prototype
platform/x86: x86-android-tablets: Add support for getting
serdev-controller by PCI parent
platform/x86: x86-android-tablets: Add Bluetooth support for Vexia EDU
ATLA 10
drivers/platform/x86/serdev_helpers.h | 60 +++++++++++--------
.../platform/x86/x86-android-tablets/core.c | 31 +++++++---
.../platform/x86/x86-android-tablets/other.c | 12 +++-
.../x86-android-tablets/x86-android-tablets.h | 6 +-
4 files changed, 74 insertions(+), 35 deletions(-)
--
2.47.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/5] platform/x86: serdev_helpers: Add get_serdev_controller_from_parent() helper
2024-11-09 22:05 [PATCH 0/5] platform/x86: x86-android-tablets: Add Bluetooth support for Vexia EDU ATLA 10 Hans de Goede
@ 2024-11-09 22:05 ` Hans de Goede
2024-11-09 22:05 ` [PATCH 2/5] platform/x86: x86-android-tablets: Add missing __init to get_i2c_adap_by_*() Hans de Goede
` (4 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Hans de Goede @ 2024-11-09 22:05 UTC (permalink / raw)
To: Ilpo Järvinen, Andy Shevchenko; +Cc: Hans de Goede, platform-driver-x86
The x86-android-tablets code needs to be able to get a serdev_controller
device from a PCI parent, rather then by the ACPI HID+UID of the parent,
because on some tablets the UARTs are enumerated as PCI devices instead
of ACPI devices.
Split the code to walk the device hierarchy to find the serdev_controller
from its parents out into a get_serdev_controller_from_parent() helper
so that the x86-android-tablets code can re-use it.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/platform/x86/serdev_helpers.h | 60 +++++++++++++++------------
1 file changed, 34 insertions(+), 26 deletions(-)
diff --git a/drivers/platform/x86/serdev_helpers.h b/drivers/platform/x86/serdev_helpers.h
index bcf3a0c356ea..b592b9ff6d93 100644
--- a/drivers/platform/x86/serdev_helpers.h
+++ b/drivers/platform/x86/serdev_helpers.h
@@ -22,32 +22,14 @@
#include <linux/string.h>
static inline struct device *
-get_serdev_controller(const char *serial_ctrl_hid,
- const char *serial_ctrl_uid,
- int serial_ctrl_port,
- const char *serdev_ctrl_name)
+get_serdev_controller_from_parent(struct device *ctrl_dev,
+ int serial_ctrl_port,
+ const char *serdev_ctrl_name)
{
- struct device *ctrl_dev, *child;
- struct acpi_device *ctrl_adev;
+ struct device *child;
char name[32];
int i;
- ctrl_adev = acpi_dev_get_first_match_dev(serial_ctrl_hid, serial_ctrl_uid, -1);
- if (!ctrl_adev) {
- pr_err("error could not get %s/%s serial-ctrl adev\n",
- serial_ctrl_hid, serial_ctrl_uid);
- return ERR_PTR(-ENODEV);
- }
-
- /* get_first_physical_node() returns a weak ref */
- ctrl_dev = get_device(acpi_get_first_physical_node(ctrl_adev));
- if (!ctrl_dev) {
- pr_err("error could not get %s/%s serial-ctrl physical node\n",
- serial_ctrl_hid, serial_ctrl_uid);
- ctrl_dev = ERR_PTR(-ENODEV);
- goto put_ctrl_adev;
- }
-
/* Walk host -> uart-ctrl -> port -> serdev-ctrl */
for (i = 0; i < 3; i++) {
switch (i) {
@@ -67,14 +49,40 @@ get_serdev_controller(const char *serial_ctrl_hid,
put_device(ctrl_dev);
if (!child) {
pr_err("error could not find '%s' device\n", name);
- ctrl_dev = ERR_PTR(-ENODEV);
- goto put_ctrl_adev;
+ return ERR_PTR(-ENODEV);
}
ctrl_dev = child;
}
-put_ctrl_adev:
- acpi_dev_put(ctrl_adev);
return ctrl_dev;
}
+
+static inline struct device *
+get_serdev_controller(const char *serial_ctrl_hid,
+ const char *serial_ctrl_uid,
+ int serial_ctrl_port,
+ const char *serdev_ctrl_name)
+{
+ struct acpi_device *adev;
+ struct device *parent;
+
+ adev = acpi_dev_get_first_match_dev(serial_ctrl_hid, serial_ctrl_uid, -1);
+ if (!adev) {
+ pr_err("error could not get %s/%s serial-ctrl adev\n",
+ serial_ctrl_hid, serial_ctrl_uid);
+ return ERR_PTR(-ENODEV);
+ }
+
+ /* get_first_physical_node() returns a weak ref */
+ parent = get_device(acpi_get_first_physical_node(adev));
+ acpi_dev_put(adev);
+ if (!parent) {
+ pr_err("error could not get %s/%s serial-ctrl physical node\n",
+ serial_ctrl_hid, serial_ctrl_uid);
+ return ERR_PTR(-ENODEV);
+ }
+
+ /* This puts our reference on parent and returns a ref on the ctrl */
+ return get_serdev_controller_from_parent(parent, serial_ctrl_port, serdev_ctrl_name);
+}
--
2.47.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/5] platform/x86: x86-android-tablets: Add missing __init to get_i2c_adap_by_*()
2024-11-09 22:05 [PATCH 0/5] platform/x86: x86-android-tablets: Add Bluetooth support for Vexia EDU ATLA 10 Hans de Goede
2024-11-09 22:05 ` [PATCH 1/5] platform/x86: serdev_helpers: Add get_serdev_controller_from_parent() helper Hans de Goede
@ 2024-11-09 22:05 ` Hans de Goede
2024-11-09 22:05 ` [PATCH 3/5] platform/x86: x86-android-tablets: Change x86_instantiate_serdev() prototype Hans de Goede
` (3 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Hans de Goede @ 2024-11-09 22:05 UTC (permalink / raw)
To: Ilpo Järvinen, Andy Shevchenko; +Cc: Hans de Goede, platform-driver-x86
get_i2c_adap_by_handle() and get_i2c_adap_by_pci_parent() both are only
used by x86_instantiate_i2c_client() which is __init itself and in case
of the latter it also uses match_parent() which is also __init.
Fixes: 5b78e809f948 ("platform/x86: x86-android-tablets: Add support for getting i2c_adapter by PCI parent devname()")
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/platform/x86/x86-android-tablets/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/x86-android-tablets/core.c b/drivers/platform/x86/x86-android-tablets/core.c
index e9f4d91496d5..affaffadd179 100644
--- a/drivers/platform/x86/x86-android-tablets/core.c
+++ b/drivers/platform/x86/x86-android-tablets/core.c
@@ -157,7 +157,7 @@ static struct gpiod_lookup_table * const *gpiod_lookup_tables;
static const struct software_node *bat_swnode;
static void (*exit_handler)(void);
-static struct i2c_adapter *
+static __init struct i2c_adapter *
get_i2c_adap_by_handle(const struct x86_i2c_client_info *client_info)
{
acpi_handle handle;
@@ -177,7 +177,7 @@ static __init int match_parent(struct device *dev, const void *data)
return dev->parent == data;
}
-static struct i2c_adapter *
+static __init struct i2c_adapter *
get_i2c_adap_by_pci_parent(const struct x86_i2c_client_info *client_info)
{
struct i2c_adapter *adap = NULL;
--
2.47.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/5] platform/x86: x86-android-tablets: Change x86_instantiate_serdev() prototype
2024-11-09 22:05 [PATCH 0/5] platform/x86: x86-android-tablets: Add Bluetooth support for Vexia EDU ATLA 10 Hans de Goede
2024-11-09 22:05 ` [PATCH 1/5] platform/x86: serdev_helpers: Add get_serdev_controller_from_parent() helper Hans de Goede
2024-11-09 22:05 ` [PATCH 2/5] platform/x86: x86-android-tablets: Add missing __init to get_i2c_adap_by_*() Hans de Goede
@ 2024-11-09 22:05 ` Hans de Goede
2024-11-09 22:05 ` [PATCH 4/5] platform/x86: x86-android-tablets: Add support for getting serdev-controller by PCI parent Hans de Goede
` (2 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Hans de Goede @ 2024-11-09 22:05 UTC (permalink / raw)
To: Ilpo Järvinen, Andy Shevchenko; +Cc: Hans de Goede, platform-driver-x86
Make x86_instantiate_serdev() take a "struct x86_dev_info *" + idx as
arguments instead of a "struct x86_serdev_info *" + idx.
This makes the x86_instantiate_serdev() prototype match
the x86_instantiate_i2c_client() and x86_instantiate_spi_dev() prototypes.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/platform/x86/x86-android-tablets/core.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/x86-android-tablets/core.c b/drivers/platform/x86/x86-android-tablets/core.c
index affaffadd179..800d6c84dced 100644
--- a/drivers/platform/x86/x86-android-tablets/core.c
+++ b/drivers/platform/x86/x86-android-tablets/core.c
@@ -271,8 +271,9 @@ static __init int x86_instantiate_spi_dev(const struct x86_dev_info *dev_info, i
return 0;
}
-static __init int x86_instantiate_serdev(const struct x86_serdev_info *info, int idx)
+static __init int x86_instantiate_serdev(const struct x86_dev_info *dev_info, int idx)
{
+ const struct x86_serdev_info *info = &dev_info->serdev_info[idx];
struct acpi_device *serdev_adev;
struct serdev_device *serdev;
struct device *ctrl_dev;
@@ -446,7 +447,7 @@ static __init int x86_android_tablet_probe(struct platform_device *pdev)
serdev_count = dev_info->serdev_count;
for (i = 0; i < serdev_count; i++) {
- ret = x86_instantiate_serdev(&dev_info->serdev_info[i], i);
+ ret = x86_instantiate_serdev(dev_info, i);
if (ret < 0) {
x86_android_tablet_remove(pdev);
return ret;
--
2.47.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 4/5] platform/x86: x86-android-tablets: Add support for getting serdev-controller by PCI parent
2024-11-09 22:05 [PATCH 0/5] platform/x86: x86-android-tablets: Add Bluetooth support for Vexia EDU ATLA 10 Hans de Goede
` (2 preceding siblings ...)
2024-11-09 22:05 ` [PATCH 3/5] platform/x86: x86-android-tablets: Change x86_instantiate_serdev() prototype Hans de Goede
@ 2024-11-09 22:05 ` Hans de Goede
2024-11-10 11:51 ` Andy Shevchenko
2024-11-09 22:05 ` [PATCH 5/5] platform/x86: x86-android-tablets: Add Bluetooth support for Vexia EDU ATLA 10 Hans de Goede
2024-11-10 14:53 ` [PATCH 0/5] " Andy Shevchenko
5 siblings, 1 reply; 11+ messages in thread
From: Hans de Goede @ 2024-11-09 22:05 UTC (permalink / raw)
To: Ilpo Järvinen, Andy Shevchenko; +Cc: Hans de Goede, platform-driver-x86
On the Vexia EDU ATLA 10 tablet, which ships with Android + a custom Linux
(guadalinex) using the custom Android kernel the UART controllers are not
enumerated as ACPI devices as they typically are.
Instead they are enumerated through PCI and getting the serdev-controller
by ACPI HID + UID does not work.
Add support for getting the serdev-controller by the PCI devfn of its
parent instead.
This also renames the use_pci_devname flag to use_pci since the former
name now no longer is accurate.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
.../platform/x86/x86-android-tablets/core.c | 22 ++++++++++++++++---
.../platform/x86/x86-android-tablets/other.c | 2 +-
.../x86-android-tablets/x86-android-tablets.h | 6 ++++-
3 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/drivers/platform/x86/x86-android-tablets/core.c b/drivers/platform/x86/x86-android-tablets/core.c
index 800d6c84dced..4633a9560bd2 100644
--- a/drivers/platform/x86/x86-android-tablets/core.c
+++ b/drivers/platform/x86/x86-android-tablets/core.c
@@ -212,7 +212,7 @@ static __init int x86_instantiate_i2c_client(const struct x86_dev_info *dev_info
if (board_info.irq < 0)
return board_info.irq;
- if (dev_info->use_pci_devname)
+ if (dev_info->use_pci)
adap = get_i2c_adap_by_pci_parent(client_info);
else
adap = get_i2c_adap_by_handle(client_info);
@@ -271,6 +271,19 @@ static __init int x86_instantiate_spi_dev(const struct x86_dev_info *dev_info, i
return 0;
}
+static __init struct device *
+get_serdev_controller_by_pci_parent(const struct x86_serdev_info *info)
+{
+ struct pci_dev *pdev;
+
+ pdev = pci_get_domain_bus_and_slot(0, 0, info->ctrl_devfn);
+ if (!pdev)
+ return ERR_PTR(-EPROBE_DEFER);
+
+ /* This puts our reference on pdev and returns a ref on the ctrl */
+ return get_serdev_controller_from_parent(&pdev->dev, 0, info->ctrl_devname);
+}
+
static __init int x86_instantiate_serdev(const struct x86_dev_info *dev_info, int idx)
{
const struct x86_serdev_info *info = &dev_info->serdev_info[idx];
@@ -279,8 +292,11 @@ static __init int x86_instantiate_serdev(const struct x86_dev_info *dev_info, in
struct device *ctrl_dev;
int ret = -ENODEV;
- ctrl_dev = get_serdev_controller(info->ctrl_hid, info->ctrl_uid, 0,
- info->ctrl_devname);
+ if (dev_info->use_pci)
+ ctrl_dev = get_serdev_controller_by_pci_parent(info);
+ else
+ ctrl_dev = get_serdev_controller(info->ctrl_hid, info->ctrl_uid, 0,
+ info->ctrl_devname);
if (IS_ERR(ctrl_dev))
return PTR_ERR(ctrl_dev);
diff --git a/drivers/platform/x86/x86-android-tablets/other.c b/drivers/platform/x86/x86-android-tablets/other.c
index 725948044da4..f5140d5ce61a 100644
--- a/drivers/platform/x86/x86-android-tablets/other.c
+++ b/drivers/platform/x86/x86-android-tablets/other.c
@@ -757,7 +757,7 @@ const struct x86_dev_info vexia_edu_atla10_info __initconst = {
.i2c_client_count = ARRAY_SIZE(vexia_edu_atla10_i2c_clients),
.gpiod_lookup_tables = vexia_edu_atla10_gpios,
.init = vexia_edu_atla10_init,
- .use_pci_devname = true,
+ .use_pci = true,
};
/*
diff --git a/drivers/platform/x86/x86-android-tablets/x86-android-tablets.h b/drivers/platform/x86/x86-android-tablets/x86-android-tablets.h
index 0fc7e8cff672..bab27fdcd873 100644
--- a/drivers/platform/x86/x86-android-tablets/x86-android-tablets.h
+++ b/drivers/platform/x86/x86-android-tablets/x86-android-tablets.h
@@ -57,8 +57,12 @@ struct x86_spi_dev_info {
};
struct x86_serdev_info {
+ /* For ACPI enumerated controllers */
const char *ctrl_hid;
const char *ctrl_uid;
+ /* For PCI enumerated controllers */
+ unsigned int ctrl_devfn;
+ /* Typically "serial0" */
const char *ctrl_devname;
/*
* ATM the serdev core only supports of or ACPI matching; and so far all
@@ -91,7 +95,7 @@ struct x86_dev_info {
int gpio_button_count;
int (*init)(struct device *dev);
void (*exit)(void);
- bool use_pci_devname;
+ bool use_pci;
};
int x86_android_tablet_get_gpiod(const char *chip, int pin, const char *con_id,
--
2.47.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 5/5] platform/x86: x86-android-tablets: Add Bluetooth support for Vexia EDU ATLA 10
2024-11-09 22:05 [PATCH 0/5] platform/x86: x86-android-tablets: Add Bluetooth support for Vexia EDU ATLA 10 Hans de Goede
` (3 preceding siblings ...)
2024-11-09 22:05 ` [PATCH 4/5] platform/x86: x86-android-tablets: Add support for getting serdev-controller by PCI parent Hans de Goede
@ 2024-11-09 22:05 ` Hans de Goede
2024-11-10 14:53 ` [PATCH 0/5] " Andy Shevchenko
5 siblings, 0 replies; 11+ messages in thread
From: Hans de Goede @ 2024-11-09 22:05 UTC (permalink / raw)
To: Ilpo Järvinen, Andy Shevchenko; +Cc: Hans de Goede, platform-driver-x86
The UART used for the Bluetooth HCI on the Vexia EDU ATLA 10 is enumerated
as a PCI device, but the ODBA7823 ACPI fwnode for the HCI expects it to
use the more standard ACPI enumeration mode.
So Bluetooth does not work out of the box. Add x86_serdev_info to make
the x86-android-tablets manually associate the fwnode with the UART.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/platform/x86/x86-android-tablets/other.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/platform/x86/x86-android-tablets/other.c b/drivers/platform/x86/x86-android-tablets/other.c
index f5140d5ce61a..9b8353dec7b6 100644
--- a/drivers/platform/x86/x86-android-tablets/other.c
+++ b/drivers/platform/x86/x86-android-tablets/other.c
@@ -715,6 +715,14 @@ static const struct x86_i2c_client_info vexia_edu_atla10_i2c_clients[] __initcon
}
};
+static const struct x86_serdev_info vexia_edu_atla10_serdevs[] __initconst = {
+ {
+ .ctrl_devfn = PCI_DEVFN(0x1e, 3),
+ .ctrl_devname = "serial0",
+ .serdev_hid = "OBDA8723",
+ },
+};
+
static struct gpiod_lookup_table vexia_edu_atla10_ft5416_gpios = {
.dev_id = "i2c-FTSC1000",
.table = {
@@ -755,6 +763,8 @@ static int __init vexia_edu_atla10_init(struct device *dev)
const struct x86_dev_info vexia_edu_atla10_info __initconst = {
.i2c_client_info = vexia_edu_atla10_i2c_clients,
.i2c_client_count = ARRAY_SIZE(vexia_edu_atla10_i2c_clients),
+ .serdev_info = vexia_edu_atla10_serdevs,
+ .serdev_count = ARRAY_SIZE(vexia_edu_atla10_serdevs),
.gpiod_lookup_tables = vexia_edu_atla10_gpios,
.init = vexia_edu_atla10_init,
.use_pci = true,
--
2.47.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 4/5] platform/x86: x86-android-tablets: Add support for getting serdev-controller by PCI parent
2024-11-09 22:05 ` [PATCH 4/5] platform/x86: x86-android-tablets: Add support for getting serdev-controller by PCI parent Hans de Goede
@ 2024-11-10 11:51 ` Andy Shevchenko
2024-11-10 22:27 ` Hans de Goede
0 siblings, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2024-11-10 11:51 UTC (permalink / raw)
To: Hans de Goede; +Cc: Ilpo Järvinen, Andy Shevchenko, platform-driver-x86
On Sun, Nov 10, 2024 at 12:05 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> On the Vexia EDU ATLA 10 tablet, which ships with Android + a custom Linux
> (guadalinex) using the custom Android kernel the UART controllers are not
> enumerated as ACPI devices as they typically are.
>
> Instead they are enumerated through PCI and getting the serdev-controller
> by ACPI HID + UID does not work.
>
> Add support for getting the serdev-controller by the PCI devfn of its
> parent instead.
>
> This also renames the use_pci_devname flag to use_pci since the former
> name now no longer is accurate.
...
> + if (dev_info->use_pci)
> + ctrl_dev = get_serdev_controller_by_pci_parent(info);
> + else
> + ctrl_dev = get_serdev_controller(info->ctrl_hid, info->ctrl_uid, 0,
> + info->ctrl_devname);
I would expect that they both take info as an argument...
> if (IS_ERR(ctrl_dev))
> return PTR_ERR(ctrl_dev);
...
> struct x86_serdev_info {
> + /* For ACPI enumerated controllers */
> const char *ctrl_hid;
> const char *ctrl_uid;
> + /* For PCI enumerated controllers */
> + unsigned int ctrl_devfn;
> + /* Typically "serial0" */
> const char *ctrl_devname;
Why not union as we have a type selector, i.e. use_pci ?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/5] platform/x86: x86-android-tablets: Add Bluetooth support for Vexia EDU ATLA 10
2024-11-09 22:05 [PATCH 0/5] platform/x86: x86-android-tablets: Add Bluetooth support for Vexia EDU ATLA 10 Hans de Goede
` (4 preceding siblings ...)
2024-11-09 22:05 ` [PATCH 5/5] platform/x86: x86-android-tablets: Add Bluetooth support for Vexia EDU ATLA 10 Hans de Goede
@ 2024-11-10 14:53 ` Andy Shevchenko
5 siblings, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2024-11-10 14:53 UTC (permalink / raw)
To: Hans de Goede; +Cc: Ilpo Järvinen, Andy Shevchenko, platform-driver-x86
On Sun, Nov 10, 2024 at 12:05 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi All,
>
> Here is a patch-series for adding Bluetooth support for the Vexia EDU ATLA
> 10 tablet to x86-android-tablets.
>
> Due to the LPSS UARTs being enumerated through PCI rather then through
rather than
> ACPI, this is somewhat involved. Just like how this special case needed
> some extra work for instantiating the various i2c-clients.
All seem reasonable to me
Reviewed-by: Andy Shevchenko <andy@kernel.org>
One patch got a few comments, and I still think it's better to move
the rather long functions to the C file at some point.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 4/5] platform/x86: x86-android-tablets: Add support for getting serdev-controller by PCI parent
2024-11-10 11:51 ` Andy Shevchenko
@ 2024-11-10 22:27 ` Hans de Goede
2024-11-11 9:37 ` Andy Shevchenko
0 siblings, 1 reply; 11+ messages in thread
From: Hans de Goede @ 2024-11-10 22:27 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Ilpo Järvinen, Andy Shevchenko, platform-driver-x86
Hi,
On 10-Nov-24 12:51 PM, Andy Shevchenko wrote:
> On Sun, Nov 10, 2024 at 12:05 AM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> On the Vexia EDU ATLA 10 tablet, which ships with Android + a custom Linux
>> (guadalinex) using the custom Android kernel the UART controllers are not
>> enumerated as ACPI devices as they typically are.
>>
>> Instead they are enumerated through PCI and getting the serdev-controller
>> by ACPI HID + UID does not work.
>>
>> Add support for getting the serdev-controller by the PCI devfn of its
>> parent instead.
>>
>> This also renames the use_pci_devname flag to use_pci since the former
>> name now no longer is accurate.
>
> ...
>
>> + if (dev_info->use_pci)
>> + ctrl_dev = get_serdev_controller_by_pci_parent(info);
>> + else
>> + ctrl_dev = get_serdev_controller(info->ctrl_hid, info->ctrl_uid, 0,
>> + info->ctrl_devname);
>
> I would expect that they both take info as an argument...
The "old" get_serdev_controller() helper for getting the controller
by ACPI HID + UID is also used outside of the x86-android-tablets code
and struct x86_serdev_info is x86-android-tablets specific.
>> if (IS_ERR(ctrl_dev))
>> return PTR_ERR(ctrl_dev);
>
> ...
>
>> struct x86_serdev_info {
>> + /* For ACPI enumerated controllers */
>> const char *ctrl_hid;
>> const char *ctrl_uid;
>> + /* For PCI enumerated controllers */
>> + unsigned int ctrl_devfn;
>> + /* Typically "serial0" */
>> const char *ctrl_devname;
>
> Why not union as we have a type selector, i.e. use_pci ?
A union would be possible. I simply did not think of that.
Not sure if it is worth the trouble though, since it saves
only 8 bytes on a struct which is currently used only 3
times in the driver.
Regards,
Hans
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 4/5] platform/x86: x86-android-tablets: Add support for getting serdev-controller by PCI parent
2024-11-10 22:27 ` Hans de Goede
@ 2024-11-11 9:37 ` Andy Shevchenko
2024-11-11 9:53 ` Hans de Goede
0 siblings, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2024-11-11 9:37 UTC (permalink / raw)
To: Hans de Goede; +Cc: Ilpo Järvinen, platform-driver-x86
On Sun, Nov 10, 2024 at 11:27:31PM +0100, Hans de Goede wrote:
> On 10-Nov-24 12:51 PM, Andy Shevchenko wrote:
> > On Sun, Nov 10, 2024 at 12:05 AM Hans de Goede <hdegoede@redhat.com> wrote:
...
> >> + if (dev_info->use_pci)
> >> + ctrl_dev = get_serdev_controller_by_pci_parent(info);
> >> + else
> >> + ctrl_dev = get_serdev_controller(info->ctrl_hid, info->ctrl_uid, 0,
> >> + info->ctrl_devname);
> >
> > I would expect that they both take info as an argument...
>
> The "old" get_serdev_controller() helper for getting the controller
> by ACPI HID + UID is also used outside of the x86-android-tablets code
> and struct x86_serdev_info is x86-android-tablets specific.
I see, thanks for elaborating this.
> >> if (IS_ERR(ctrl_dev))
> >> return PTR_ERR(ctrl_dev);
...
> >> struct x86_serdev_info {
> >> + /* For ACPI enumerated controllers */
> >> const char *ctrl_hid;
> >> const char *ctrl_uid;
> >> + /* For PCI enumerated controllers */
> >> + unsigned int ctrl_devfn;
> >> + /* Typically "serial0" */
> >> const char *ctrl_devname;
> >
> > Why not union as we have a type selector, i.e. use_pci ?
>
> A union would be possible. I simply did not think of that.
>
> Not sure if it is worth the trouble though, since it saves
> only 8 bytes on a struct which is currently used only 3
> times in the driver.
Saving bytes is not the main point, the main point is to make the code robust
against writing both fields and make things confusing. Along with that, union
gives a reader the clue that those fields are never used at the same time. So,
can you consider using union?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 4/5] platform/x86: x86-android-tablets: Add support for getting serdev-controller by PCI parent
2024-11-11 9:37 ` Andy Shevchenko
@ 2024-11-11 9:53 ` Hans de Goede
0 siblings, 0 replies; 11+ messages in thread
From: Hans de Goede @ 2024-11-11 9:53 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Ilpo Järvinen, platform-driver-x86
Hi,
On 11-Nov-24 10:37 AM, Andy Shevchenko wrote:
> On Sun, Nov 10, 2024 at 11:27:31PM +0100, Hans de Goede wrote:
>> On 10-Nov-24 12:51 PM, Andy Shevchenko wrote:
>>> On Sun, Nov 10, 2024 at 12:05 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> ...
>
>>>> + if (dev_info->use_pci)
>>>> + ctrl_dev = get_serdev_controller_by_pci_parent(info);
>>>> + else
>>>> + ctrl_dev = get_serdev_controller(info->ctrl_hid, info->ctrl_uid, 0,
>>>> + info->ctrl_devname);
>>>
>>> I would expect that they both take info as an argument...
>>
>> The "old" get_serdev_controller() helper for getting the controller
>> by ACPI HID + UID is also used outside of the x86-android-tablets code
>> and struct x86_serdev_info is x86-android-tablets specific.
>
> I see, thanks for elaborating this.
>
>>>> if (IS_ERR(ctrl_dev))
>>>> return PTR_ERR(ctrl_dev);
>
> ...
>
>>>> struct x86_serdev_info {
>>>> + /* For ACPI enumerated controllers */
>>>> const char *ctrl_hid;
>>>> const char *ctrl_uid;
>>>> + /* For PCI enumerated controllers */
>>>> + unsigned int ctrl_devfn;
>>>> + /* Typically "serial0" */
>>>> const char *ctrl_devname;
>>>
>>> Why not union as we have a type selector, i.e. use_pci ?
>>
>> A union would be possible. I simply did not think of that.
>>
>> Not sure if it is worth the trouble though, since it saves
>> only 8 bytes on a struct which is currently used only 3
>> times in the driver.
>
> Saving bytes is not the main point, the main point is to make the code robust
> against writing both fields and make things confusing. Along with that, union
> gives a reader the clue that those fields are never used at the same time. So,
> can you consider using union?
Ack, I will prepare a v2 using an union when I have some time to work on this.
Regards,
Hans
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-11-11 9:53 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-09 22:05 [PATCH 0/5] platform/x86: x86-android-tablets: Add Bluetooth support for Vexia EDU ATLA 10 Hans de Goede
2024-11-09 22:05 ` [PATCH 1/5] platform/x86: serdev_helpers: Add get_serdev_controller_from_parent() helper Hans de Goede
2024-11-09 22:05 ` [PATCH 2/5] platform/x86: x86-android-tablets: Add missing __init to get_i2c_adap_by_*() Hans de Goede
2024-11-09 22:05 ` [PATCH 3/5] platform/x86: x86-android-tablets: Change x86_instantiate_serdev() prototype Hans de Goede
2024-11-09 22:05 ` [PATCH 4/5] platform/x86: x86-android-tablets: Add support for getting serdev-controller by PCI parent Hans de Goede
2024-11-10 11:51 ` Andy Shevchenko
2024-11-10 22:27 ` Hans de Goede
2024-11-11 9:37 ` Andy Shevchenko
2024-11-11 9:53 ` Hans de Goede
2024-11-09 22:05 ` [PATCH 5/5] platform/x86: x86-android-tablets: Add Bluetooth support for Vexia EDU ATLA 10 Hans de Goede
2024-11-10 14:53 ` [PATCH 0/5] " Andy Shevchenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox