Patchwork [4/5] ARM: DT: tegra114: add KBC controller DT entry

login
register
mail settings
Submitter Laxman Dewangan
Date March 8, 2013, 1:53 p.m.
Message ID <1362750782-15174-5-git-send-email-ldewangan@nvidia.com>
Download mbox | patch
Permalink /patch/226123/
State Superseded, archived
Headers show

Comments

Laxman Dewangan - March 8, 2013, 1:53 p.m.
NVIDIA's Tegra114 SoCs have the matrix keyboard controller which
supports 11x8 type of matrix. The number of rows and columns
are configurable.

Add DT entry for KBC controller.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 arch/arm/boot/dts/tegra114.dtsi |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)
Stephen Warren - March 8, 2013, 6:04 p.m.
On 03/08/2013 06:53 AM, Laxman Dewangan wrote:
> NVIDIA's Tegra114 SoCs have the matrix keyboard controller which
> supports 11x8 type of matrix. The number of rows and columns
> are configurable.

Earlier Tegra versions supported up to a 16x8 matrix. This feeds into
the following defines in the driver:

#define KBC_MAX_GPIO    24
#define KBC_MAX_ROW     16
#define KBC_MAX_COL     8
#define KBC_MAX_KEY     (KBC_MAX_ROW * KBC_MAX_COL)

Given Tegra114 supports /fewer/ pins and rows than earlier chips, I
think that makes the HW technically incompatible, since GPIO IDs 19..23
are invalid in this HW but valid earlier.

Now in practice I suppose that with a correct DT keyboard map for a
Tegra114 device, those extra invalid GPIOs would never be referenced, so
this is a little nit-picky, but I still feel we should fix this.

So, I'd like to see the KBC driver updated to derive the values for all
the defines I listed above from the compatible value.

Technically, tegra114.dtsi should not pretend that the Tegra114 KBC is
compatible with previous generations either.

> diff --git a/arch/arm/boot/dts/tegra114.dtsi b/arch/arm/boot/dts/tegra114.dtsi

> +	kbc {
> +		compatible = "nvidia,tegra20-kbc";

Ignoring any of the discussion above, i.e. even if the HW was 100%
identical, we should still include a Tegra114 entry in the compatible
property (compatible = "nvidia,tegra114-kbc", "nvidia,tegra20-kbc"), so
that if Tegra114-specific bugs were found in the future, any device
trees would already indicate that they describe Tegra114 HW, so the SW
could key off that to enable the workaround.

Re-stated: The rules for compatible are: Always include the exact HW
name, then optionally include any other HW names it's compatible with.

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Laxman Dewangan - March 8, 2013, 6:14 p.m.
On Friday 08 March 2013 11:34 PM, Stephen Warren wrote:
> On 03/08/2013 06:53 AM, Laxman Dewangan wrote:
>> NVIDIA's Tegra114 SoCs have the matrix keyboard controller which
>> supports 11x8 type of matrix. The number of rows and columns
>> are configurable.
> Earlier Tegra versions supported up to a 16x8 matrix. This feeds into
> the following defines in the driver:
>
> #define KBC_MAX_GPIO    24
> #define KBC_MAX_ROW     16
> #define KBC_MAX_COL     8
> #define KBC_MAX_KEY     (KBC_MAX_ROW * KBC_MAX_COL)
>
> Given Tegra114 supports /fewer/ pins and rows than earlier chips, I
> think that makes the HW technically incompatible, since GPIO IDs 19..23
> are invalid in this HW but valid earlier.
>
> Now in practice I suppose that with a correct DT keyboard map for a
> Tegra114 device, those extra invalid GPIOs would never be referenced, so
> this is a little nit-picky, but I still feel we should fix this.

Where do we fix this? In binding document?

>
> So, I'd like to see the KBC driver updated to derive the values for all
> the defines I listed above from the compatible value.

Ok, we can update the kbc driver to derive above param from compatible.

>
>
>
> Re-stated: The rules for compatible are: Always include the exact HW
> name, then optionally include any other HW names it's compatible with.

OK, will update this in next version of patch.
>

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Warren - March 8, 2013, 6:42 p.m.
On 03/08/2013 11:14 AM, Laxman Dewangan wrote:
> On Friday 08 March 2013 11:34 PM, Stephen Warren wrote:
>> On 03/08/2013 06:53 AM, Laxman Dewangan wrote:
>>> NVIDIA's Tegra114 SoCs have the matrix keyboard controller which
>>> supports 11x8 type of matrix. The number of rows and columns
>>> are configurable.
>> Earlier Tegra versions supported up to a 16x8 matrix. This feeds into
>> the following defines in the driver:
>>
>> #define KBC_MAX_GPIO    24
>> #define KBC_MAX_ROW     16
>> #define KBC_MAX_COL     8
>> #define KBC_MAX_KEY     (KBC_MAX_ROW * KBC_MAX_COL)
>>
>> Given Tegra114 supports /fewer/ pins and rows than earlier chips, I
>> think that makes the HW technically incompatible, since GPIO IDs 19..23
>> are invalid in this HW but valid earlier.
>>
>> Now in practice I suppose that with a correct DT keyboard map for a
>> Tegra114 device, those extra invalid GPIOs would never be referenced, so
>> this is a little nit-picky, but I still feel we should fix this.
> 
> Where do we fix this? In binding document?

In the driver and .dtsi files; the rest of my message was describing how
to fix this.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/arch/arm/boot/dts/tegra114.dtsi b/arch/arm/boot/dts/tegra114.dtsi
index 686e33f..18fbd49 100644
--- a/arch/arm/boot/dts/tegra114.dtsi
+++ b/arch/arm/boot/dts/tegra114.dtsi
@@ -248,6 +248,14 @@ 
 		clocks = <&tegra_car 4>;
 	};
 
+	kbc {
+		compatible = "nvidia,tegra20-kbc";
+		reg = <0x7000e200 0x100>;
+		interrupts = <0 85 0x04>;
+		clocks = <&tegra_car 36>;
+		status = "disabled";
+	};
+
 	pmc {
 		compatible = "nvidia,tegra114-pmc";
 		reg = <0x7000e400 0x400>;