Re: [PATCH 2/2] platform/x86: x86-android-tablets: Add support for Vexia EDU ATLA 10 tablet

[Hans de Goede <hdegoede@redhat.com>]

Hi Andy,

On 25-Oct-24 7:47 PM, Andy Shevchenko wrote:
> On Fri, Oct 25, 2024 at 11:44:35AM +0200, Hans de Goede wrote:
>> Add support for the Vexia EDU ATLA 10 tablet, Android 4.2/4.4 + Guadalinex
>> Ubuntu tablet distributed to schools in the Spanish Andalucía region.
>>
>> Besides the usual broken DSDT issues this tablet is special because all
>> its LPSS island peripherals are enumerated as PCI devices rather then as
>> ACPI devices as they typically are.
> 
> Just wondering if you investigated the possibility to switch this via
> (most likely hidden or absent) BIOS options? On the prototypes we have
> a switch in the firmware on how to enumerate LPSS, so it's possible
> to get them enumerated as ACPI ones.

I checked (before sending out these patches) and the option to switch
LPSS devices to be ACPI enumerated is there (it as an unlocked BIOS with
all options) but it does not work (it does not do anything at all).

> 
>> At the same time there are disabled (_STA=0) ACPI devices for
>> the peripherals and child ACPI devices for e.g. attached I2C/SDIO devices
>> are children of these disabled ACPI devices and thus will not be used
>> by Linux since the parent is disabled.
>>
>> So besides the usual manual i2c-client instantiation for accel/touchscreen
>> this tablet also requires manual i2c-client instantiation for the codec
>> and for the PMIC.
>>
>> Also it seems the mainboard was designed for Windows not Android, so
>> it has an I2C attached embedded controller instead of allowing direct
>> access to the charger + fuel-gauge chips as is usual with Android boards.
>>
>> Normally when there is an embedded controller, there also is ACPI battery
>> support, but since this shipped with Android that is missing and Linux
>> needs to have a power_supply class driver talking directly to the EC.
> 
> ...
> 
>>  	depends on I2C && SPI && SERIAL_DEV_BUS && ACPI && EFI && GPIOLIB && PMIC_OPREGION
>> +	depends on PCI
> 
> Maybe it's time to rearrange this a bit?
> 
> 	depends on I2C && SPI && SERIAL_DEV_BUS
> 	depends on GPIOLIB && PMIC_OPREGION
> 	depends on ACPI && EFI && PCI
> 
> ?


Ack, fixed for v2; and I've also moved this to the patch to get
the i2c adapter by PCI parent since that code also needs the PCI
core.

> 
> ...
> 
>> +#include <linux/device.h>
>> +#include <linux/device/bus.h>
> 
> device.h is enough.

Ack, fixed for v2.

> 
> ...
> 
>> +static int __init vexia_edu_atla10_init(struct device *dev)
>> +{
>> +	struct device *pdev;
>> +	int ret;
>> +
>> +	/* Enable the Wifi module by setting the wifi_enable pin to 1 */
>> +	ret = x86_android_tablet_get_gpiod("INT33FC:02", 20, "wifi_enable",
>> +					   false, GPIOD_OUT_HIGH, NULL);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* Reprobe the SDIO controller to enumerate the now enabled Wifi module */
>> +	pdev = bus_find_device_by_name(&pci_bus_type, NULL, "0000:00:11.0");
>> +	if (!pdev)
>> +		return -EPROBE_DEFER;
> 
> Why? Can't the pci_get_domain_bus_and_slot() give you the same result more
> effectively?

Yes I think that that can work. I'll switch to that and run some quick tests for v2.

Regards,

Hans




> 
>> +	ret = device_reprobe(pdev);
>> +	if (ret)
>> +		dev_warn(pdev, "Reprobing 0000:00:11.0 error: %d\n", ret);
>> +
>> +	put_device(pdev);
>> +	return 0;
>> +}
>