Re: [PATCH v3 1/2] pinctrl: add support for ACPI pin function and config resources

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



>> +++ b/Documentation/driver-api/pin-control-acpi.rst
>
> We have Documentation/firmware-guide/acpi/, but I'm not sure
> which one suits better for this.

I started with firmware-guide but then moved to driver-api as I wanted to cover driver related bits as well. Let me know if it is better at firmware-guide.

+			Name (RBUF, ResourceTemplate()
+			{
+				Memory32Fixed(ReadWrite, 0x4F800000, 0x20)
+				Interrupt(ResourceConsumer, Level, ActiveHigh, Shared) {0x55}
+				PinFunction(Exclusive, PullDefault, 0x5, "\\_SB.GPI0", 0, ResourceConsumer, ) {2, 3}
+				// Configure 10k Pull up for I2C SDA/SCL pins
+				PinConfig(Exclusive, 0x01, 10000, "\\_SB.GPI0", 0, ResourceConsumer, ) {2, 3}
+			})
+			Return(RBUF)

In all examples the 2, 3 is used as a pin list for all kind of resources,
it's so confusing. Also take into account the difference between GPIO and
pin number spaces as I told before. Examples should cover that.

Also try to compile all ASL with latest ACPICA tools and fix warnings / errors.


I can try but I will have to repeat the same pin number few times to describe the pin function and config for different devices to demonstrate the pin muxing part.

+Pin control devices can add callbacks for following pinctrl_ops to handle ACPI
+pin resources.

Why? What use case requires this?
ACPI specification is more stricter in this than DT if I understand correctly
the state of affairs.  So, can't we parse the tables in the same way for all?

...

+		case PINCTRL_ACPI_PIN_FUNCTION:

+		case PINCTRL_ACPI_PIN_CONFIG:

These are definitely what we do not want to see in the individual drivers.
(I understand that it might be that some OEMs will screw up and we would
  need quirks, but not now)

Hm. Please correct me if I am wrong here. My understanding is that we need to do few mapping which only pin controller drivers can do such as ACPI function number to internal functional name or selector. I could define bindings to do those specific mappings rather than providing the current general mapping interface. Would that be better ?

+	status = acpi_get_handle(NULL, pinctrl_acpi, &handle);
+	if (ACPI_FAILURE(status))
+		return NULL;
+
+	adev = acpi_get_acpi_dev(handle);
+	if (!adev)
+		return NULL;
+
+	dev_name = acpi_dev_name(adev);
+	if (!dev_name)
+		return NULL;
+
+	return get_pinctrl_dev_from_devname(dev_name);

Are they all resource leakage-free?


I hope so. Do you see something odd ?



[Index of Archives]     [Linux SPI]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux