On Mon, Mar 01, 2021 at 09:43:29AM +0800, Shawn Guo wrote:
> It adds ACPI probe support with tile offsets passed over to msm core
> driver via sc8180x_tile_offsets, as TLMM is described a single memory
> region in ACPI DSDT.
...
> config PINCTRL_SC8180X
> tristate "Qualcomm Technologies Inc SC8180x pin controller driver"
> - depends on GPIOLIB && OF
> + depends on GPIOLIB && (OF || ACPI)
Can you consider dropping OF dependency completely?
> +#include <linux/acpi.h>
No use of this header, see below.
(Perhaps you meant mod_devicetable.h)
...
> +static const u32 sc8180x_tile_offsets[] = {
> + 0x00d00000,
> + 0x00500000,
> + 0x00100000
Leave comma here.
> +};
...
> +static const int sc8180x_acpi_reserved_gpios[] = {
> + 0, 1, 2, 3,
> + 47, 48, 49, 50,
> + 126, 127, 128, 129,
> + -1
-1?
Is it kinda terminator?
> +};
...
> + if (pdev->dev.of_node) {
> + ret = msm_pinctrl_probe(pdev, &sc8180x_pinctrl);
> + } else if (has_acpi_companion(&pdev->dev)) {
> + ret = msm_pinctrl_probe(pdev, &sc8180x_acpi_pinctrl);
> + } else {
> + dev_err(&pdev->dev, "DT and ACPI disabled\n");
> + ret = -EINVAL;
> + }
Use driver_data field for this and device_get_match_data() instead of above.
...
> +#ifdef CONFIG_ACPI
Drop this ugly ifdeffery.
> +static const struct acpi_device_id sc8180x_pinctrl_acpi_match[] = {
> + { "QCOM040D"},
> + { },
No comma for terminator line.
> +};
> +MODULE_DEVICE_TABLE(acpi, sc8180x_pinctrl_acpi_match);
> +#endif
...
> + .acpi_match_table = ACPI_PTR(sc8180x_pinctrl_acpi_match),
No ACPI_PTR(), please.
--
With Best Regards,
Andy Shevchenko
